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.

Sign up for GitHub

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

Jump to bottom

Regression (0.19.3 -> 0.20.0): trunk does not initialize wasm unless a rust asset is used #791

Closed
andoalon opened this issue May 9, 2024 · 6 comments
Closed

Regression (0.19.3 -> 0.20.0): trunk does not initialize wasm unless a rust asset is used #791

andoalon opened this issue May 9, 2024 · 6 comments
Labels
need info Further information is requested

Comments

@andoalon
Copy link
Contributor

andoalon commented May 9, 2024 •
edited
Loading

I realized that after upgrading from trunk 0.19.3 to 0.20.0 my site would not start. The background would load, but nothing would happen. I saw a suspicious warning the dev console from Firefox saying

The resource at “http://127.0.0.1:8080/frontend-ffb81958b7c6daf3_bg.wasm” preloaded with link preload was not used within a few seconds. Make sure all attributes of the preload tag are set correctly.

After digging a bit, I think I found the cause. Trunk 0.19.3 generated the following at the end of index.html:

<script type="module">
import init, * as bindings from '/frontend-ffb81958b7c6daf3.js';
init('/frontend-ffb81958b7c6daf3_bg.wasm');
window.wasmBindings = bindings;

</script>

However, trunk 0.20.0 does not generate the equivalent code unless you use a rust asset, despite the guide stating that

✅ rel="rust": [...]. This is optional. If not specified, Trunk will look for a Cargo.toml in the parent directory of the source HTML file.

I'm guessing this is due to the recent addition of the custom initializer, something probably got missed. If the intention was to make using a rust asset mandatory, the documentation is now wrong, and I would expect to have some kind of error/warning message.

Also I just noticed the guide doesn't mention the data-initializer option, this probably needs a little update
@ctron ctron added the need info Further information is requested label May 10, 2024
@ctron
Copy link
Collaborator

ctron commented May 10, 2024

I tried to reproduce it, but I can't. It works for me either way (asset or not). It would be good if you could share a reproducer.

ctron added a commit that referenced this issue May 10, 2024
@ctron
docs: mention data-initializer on the rust asset type too
c8abb41 
See #791
@andoalon
Copy link
Contributor Author

Okay, I'll prepare a minimal repro and post it later on the day 👌🏼

@andoalon
Copy link
Contributor Author

andoalon commented May 10, 2024 •
edited
Loading

That was quicker than expected.
trunk-issue-repro.zip
I included samples of index.html generated by trunk in different versions and setups in the example-generated-index.html directory.

I also noticed that 0.20.0 no longer is generating the autoreload script (or at least I don't see it)🤔. In any case, autoreload doesn't seem to be working for me in 0.20.0. I can see trunk rebuilding the code, and if I refresh the page I can see that it changed, but it doesn't automatically reload. Not sure if defaults changed or what, but otherwise that's also a bug 😅

@ctron
Copy link
Collaborator

ctron commented May 10, 2024

Ok, that was easy. You're missing a body tag. That content is appended at the end of the <body> however, there's none in your file. Adding an empty body that makes it work for me.

That's probably due to the switch from nipper to lol_html.

I guess we should either warn or inject a body that if none was found.

@andoalon
Copy link
Contributor Author

Ah, dammit. Yep, adding an empty body fixes it for me too.

Well, in that case, I would say I prefer an error, rather than tags being created magically. Or maybe making using a rust asset mandatory would also work I guess. It's more explicit so you see what is happening. And you get to specify wasm-opt options, etc. 🤔

Do as you see fit though. Thank you for taking a look!

ctron added a commit that referenced this issue May 10, 2024
@ctron
fix: handle absent body and rust-link as an error
85291f3 
If there's neither a body element, nor a link rel="rust" element, then
there's no way to guess a location for inserting the wasm initializer.

Instead of silently ignoring, or adding a body element ourselves, we
raise this as an error.

Closes #791
@ctron ctron closed this as completed in 7a484b3 May 10, 2024
@ctron
Copy link
Collaborator

ctron commented May 10, 2024

It's released as 0.20.1.

@waywardmonkeys waywardmonkeys mentioned this issue May 12, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
need info Further information is requested
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ctron @andoalon