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

fix: readme/tinygo examples use go:wasmexport/wasmimport, -buildmode=c-shared #52

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

evacchi
Copy link

@evacchi evacchi commented Jan 23, 2025

I have kept the examples with //export because this will work on both Tiny and Big Go. Once the new Go version is out we can change them all to //go:wasmexport. Notice that the -buildmode=c-shared flag and the //go:wasmexport at that point will be aligned in semantics in both compilers, see golang/go#65199

Copy link
Contributor

@mhmd-azeez mhmd-azeez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks!

README.md Outdated
@@ -68,7 +68,7 @@ platform.
Compile this with the command:

```bash
tinygo build -o plugin.wasm -target wasi main.go
tinygo build -o plugin.wasm -target wasip1 main.go
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any harm in using -buildmode c-shared here also, since this is also a reactor module

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, but the example above has a main() but you might be right because I think the issue the user on Discord had was precisely that even though they had a main TinyGo default to a reactor 🤔

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated the readme to use //go:wasmexport everywhere and -buildmode=c-shared here

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

Successfully merging this pull request may close these issues.

2 participants