Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/pkgsite: importing syscall/js breaks example formatting #67569

Closed
arvidfm opened this issue May 22, 2024 · 8 comments
Closed

x/pkgsite: importing syscall/js breaks example formatting #67569

arvidfm opened this issue May 22, 2024 · 8 comments
Labels
Milestone

Comments

@arvidfm
Copy link

arvidfm commented May 22, 2024

What is the URL of the page with the issue?

N/A

What is your user agent?

Mozilla/5.0 (X11; Linux x86_64; rv:125.0) Gecko/20100101 Firefox/125.0

Screenshot

image

What did you do?

Created a package with examples that make use of syscall/js and tried to render the documentation with pkgsite -http localhost:8080 ..

In a package named exampletest:

exampletest.go:

//go:build wasm && js

package exampletest

func Test() {}

exampletest_test.go:

//go:build wasm && js

package exampletest_test

import (
	"fmt"
	"syscall/js"
)

type A struct {
	Field string
}

func ExampleTest_one() {
	js.ValueOf(nil)
	fmt.Println(A{Field: "test1"})
	// Output:
	// {test1}
}

func ExampleTest_two() {
	fmt.Println(A{Field: "test2"})
	// Output:
	// {test2}
}

What did you see happen?

The rendered examples only include the example function body, not any imports or referenced types, functions or variables.

Note: This only seems to happen if there are at least two examples in the file. If there is only one example, it seems to (mostly) work.

What did you expect to see?

The example bodies to be playable/runnable, i.e. wrapped in a main function, and any referenced types, functions and variables to be included in the example, as seen if I comment out the use of the syscall/js package:

image

@gopherbot gopherbot added this to the Unreleased milestone May 22, 2024
@seankhliao
Copy link
Member

https://pkg.go.dev/testing

The entire test file is presented as the example when it contains a single example function, at least one other function, type, variable, or constant declaration, and no test or benchmark functions.

Unlike many projects, the Go project does not use GitHub Issues for general discussion or asking questions. GitHub Issues are used for tracking bugs and proposals only.

For questions please refer to https://github.com/golang/go/wiki/Questions

@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale May 22, 2024
@arvidfm
Copy link
Author

arvidfm commented May 22, 2024

@seankhliao Hi, thanks for the response. However, I'm not sure that your quote addresses the issue here. Please let me know if I've misunderstood, as I didn't intend this issue as a question, but as a bug report.

The entire test file is presented as the example when it contains a single example function [emphasis mine]

In other words, if there is just a single example function, pkgsite simply uses the entire file for the example. However, if there are multiple example functions, the usual behaviour is to - instead of using the entire file - extract only the symbols referenced by the example function, and to rename the example function to main. That way the example is self-contained and can be run as-is.

What I noticed with this issue, however, is that for whatever reason, if you use the syscall/js package within an example, pkgsite will only include the body of the example functions. That is, as opposed to including the symbols referenced by the function, which is the behaviour I'm used to from other - some very complex, with dozens of example functions in the same file! - example functions.

To better demonstrate the issue, I've created a minimum working example here:

https://github.com/arvidfm/pkgsite-example-mwe

You can test the MWE by cloning the repository, running pkgsite ., and navigating to the two directories within the module. Alternatively, I've requested the module to be fetched by pkg.go.dev - perhaps it will have finished processing by the time you see this.

The repository contains two packages, with_js and without_js. Aside from the naming, the packages are completely identical, with the exception that with_js makes use of the syscall/js module in one of the example functions, while without_js instead makes use of a different built-in package (database/sql/driver) in the same place. Yet despite being otherwise identical, the way that the examples from the packages are rendered is different:

with_js:
image

without_js:
image

Note how both packages contain two example functions in the same file, yet only the examples from the latter package include referenced symbols.

Please let me know if anything is unclear, or if this is actually documented behaviour that I've missed. I haven't seen anything to suggest that what modules you import in examples will affect how the examples are rendered in the documentation, but it's very possible that I've missed something!

@seankhliao
Copy link
Member

I believe that's a bug, but your expected behaviour is the wrong one.
I've filed it as #67581

@arvidfm
Copy link
Author

arvidfm commented May 22, 2024

That's not how I interpret the documentation, and either way I find the current behaviour extremely helpful, as it makes it easy to create fully runnable examples while allowing you to reuse types across examples, and I would be very sad to see it go. It would result in strictly less useful documentation.

This behaviour is also explicitly tested for upstream:

https://github.com/golang/go/blob/master/src/go/doc/testdata/examples/multiple.golden

@seankhliao
Copy link
Member

See this comment in go/doc.Examples https://go.googlesource.com/go/+/816b6031febc51692eb667df7498f3b2332987fa/src/go/doc/example.go#225

// We don't support examples that import syscall/js,
// because the package syscall/js is not available in the playground.

@arvidfm
Copy link
Author

arvidfm commented May 22, 2024

Aha! I didn't realise it was an explicit check. In that case I'll open a proposal/feature request for it instead. Thanks for taking the time to find that!

@seankhliao
Copy link
Member

Given #9679 it seems unlikely we would make it "playable".

@arvidfm
Copy link
Author

arvidfm commented May 22, 2024

Yeah, I'm less interested in actually running in the browser, and more interested in simply rendering the example with the referenced symbols included to make the examples more informative. Could e.g. just remove the play button but still render the example with full context so it can be copy-pasted as is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants