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

Playground: Add chromium support #151

Merged
merged 1 commit into from
Mar 12, 2023
Merged

Playground: Add chromium support #151

merged 1 commit into from
Mar 12, 2023

Conversation

adelhult
Copy link
Collaborator

Resolves GH-45. This PR adds support for the playground in Chromium browsers. This is done by instantiating the WASM module that contains the compiler inside of a web worker instead of the main thread.

@github-actions
Copy link

github-actions bot commented Mar 10, 2023

PR Preview Action v1.3.0
Preview removed because the pull request was closed.
2023-03-12 19:00 UTC

@CMDJojo
Copy link
Member

CMDJojo commented Mar 10, 2023

Nice! Good job on the fix! However, I think we should change to import the script using ECMAScript module syntax since it looks cleaner, doesn't pollute the scope with stuff we don't know about, and have cleaner syntax overall when calling the functions. Firefox will support it in version 111, which is released March 14th. This won't require changes to wasm-pack and so on, and is generally the preferred way to do it!

@CMDJojo
Copy link
Member

CMDJojo commented Mar 10, 2023

Also, this does not work...I think you failed to commit the --no-modules hack to get around ES module syntax. Doesn't run on FF110.0.1 on Windows 21H2

@adelhult
Copy link
Collaborator Author

Yeah, noticed that too, forced push a change a few seconds ago.

@adelhult
Copy link
Collaborator Author

Nice! Good job on the fix! However, I think we should change to import the script using ECMAScript module syntax since it looks cleaner, doesn't pollute the scope with stuff we don't know about, and have cleaner syntax overall when calling the functions. Firefox will support it in version 111, which is released March 14th. This won't require changes to wasm-pack and so on, and is generally the preferred way to do it!

Opened #152!

@CMDJojo
Copy link
Member

CMDJojo commented Mar 10, 2023

image

I would love it if you made it render on load again as it did before, as it is a nice welcoming screen when the example document gets loaded

@adelhult
Copy link
Collaborator Author

Hmm, I thought I fixed that again. Will take a look!

playground/static/main.js Outdated Show resolved Hide resolved
@adelhult adelhult force-pushed the adelhult/GH-45/2 branch 2 times, most recently from c1c1d07 to 5b4f872 Compare March 10, 2023 16:04
@CMDJojo
Copy link
Member

CMDJojo commented Mar 12, 2023

image
I still get the same problem sometimes, I think we still have a race condition between the document fetch and the parser/core loading, possibly... It would be nice to get this resolved before pushing something that works sometimes. (FF110.0.1 on Windows 21H2)

@CMDJojo
Copy link
Member

CMDJojo commented Mar 12, 2023

We might be able to solve this by fetching/displaying the welcome document strictly after we have loaded the core wasm, by sending an init event to the main (sync) JS. Or let the worker fetch the document itself

@CMDJojo
Copy link
Member

CMDJojo commented Mar 12, 2023

It seems like your last commit solved the problem. I think we are ready to merge it, but we should PR #152 asap to merge it on tuesday when the Firefox update goes stable and we can verify that ES modules work there as well! (Approved)

@adelhult adelhult merged commit 234ca3f into main Mar 12, 2023
@adelhult adelhult deleted the adelhult/GH-45/2 branch March 12, 2023 18:59
CMDJojo added a commit that referenced this pull request Mar 12, 2023
CMDJojo added a commit that referenced this pull request Mar 12, 2023
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.

Playground cannot load packages in Chromium-based browsers
2 participants