-
Notifications
You must be signed in to change notification settings - Fork 2.4k
noVNC cannot be bundled due to top-level await in browser.js #1943
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
Comments
The failure in 1.6.0 is due to the introduction of a top-level await For more context refer to novnc/noVNC#1943
Sadly had to revert back to 1.5.0 also due to this issue :/ |
I don't think modules were designed with bundling in mind, so I suspect this will always be a bit shaky. Bundling is also not something we've set out to explicitly support, so off the bat this would be an esbuild issue rather than a noVNC one. They seem to already be tracking it as evanw/esbuild#253. As for not using top level awaits; that is not something we can commit to. Most web APIs are asynchronous. Using this construct allows us to avoid a lot of complexity and bugs. I see you are using noVNC from npm, though. That is code that's been converted to CommonJS. What happens if you use the original noVNC code? |
Hm I see... So I guess the current question is, if there can be some kind of work around imagined, either possible from our side bundling it or from your side somehow. |
Because npm is primarily the domain for Node.js, which uses CommonJS rather than "regular" JavaScript. Also, since noVNC is for the web and not Node.js, npm support has always been a very secondary priority for us. I've seen that npm has grown support for ES modules, and some support for modules targeting the web. But it's not something anyone has investigated how that could be useful for noVNC.
Yes, our primary distribution form is releases on GitHub. That is what we develop and test for.
The currently problematic line is only for H.264 support, which you likely don't need. So you could try patching it out. If it works with our original code (rather than the CommonJS code in the npm package), then it could be worthwhile for someone to investigate npm's new support for ES modules. |
I see, I will try if I can figure something out regarding the .H264 Support. And on the side always learning sth new^^ As for me, "CommonJS" was always the synonym for "regular" basic Javascript and the standard to expect. Compared to that I considered this whole module-esm stuff as the the new fancy way which as you said coming slowly and might be (and is even already) supported to get loaded natively by the latest node-versions. |
I am experiencing a similiar problem when bundling using rollup, however it's error message is a bit clearer:
Which tells us that the problem is that CommonJS files don't allow top-level-await. A quick look at https://babeljs.io/docs/babel-plugin-syntax-top-level-await tells us an issue:
So the root-cause/issue is the following: Babel fails to translate ES -> CJS top-level-awaits and downstream tooling complains that top-level-awaits are not allowed in CJS, even when translating CJS -> ES
I actually use noVNC for a browser client distributed through npm and was not aware of that, it worked so well before! It looks like Node added support for ES Modules in Node 12 or so (see History), which can be enabled by setting
I will probably spend some time trying out some approaches to unblock Thymis-io/thymis#439 which broke due to the update. |
Bumps the esbuild group with 2 updates: [esbuild](https://github.com/evanw/esbuild) and [esbuild-wasm](https://github.com/evanw/esbuild). Updates `esbuild` from 0.25.0 to 0.25.1 - [Release notes](https://github.com/evanw/esbuild/releases) - [Changelog](https://github.com/evanw/esbuild/blob/main/CHANGELOG.md) - [Commits](evanw/esbuild@v0.25.0...v0.25.1) Updates `esbuild-wasm` from 0.25.0 to 0.25.1 - [Release notes](https://github.com/evanw/esbuild/releases) - [Changelog](https://github.com/evanw/esbuild/blob/main/CHANGELOG.md) - [Commits](evanw/esbuild@v0.25.0...v0.25.1) --- updated-dependencies: - dependency-name: esbuild dependency-type: direct:development update-type: version-update:semver-patch dependency-group: esbuild - dependency-name: esbuild-wasm dependency-type: direct:development update-type: version-update:semver-patch dependency-group: esbuild ... Signed-off-by: dependabot[bot] <[email protected]> Pin novnc to 1.5.0 due to novnc/noVNC#1943
Bumps the esbuild group with 2 updates: [esbuild](https://github.com/evanw/esbuild) and [esbuild-wasm](https://github.com/evanw/esbuild). Updates `esbuild` from 0.25.0 to 0.25.1 - [Release notes](https://github.com/evanw/esbuild/releases) - [Changelog](https://github.com/evanw/esbuild/blob/main/CHANGELOG.md) - [Commits](evanw/esbuild@v0.25.0...v0.25.1) Updates `esbuild-wasm` from 0.25.0 to 0.25.1 - [Release notes](https://github.com/evanw/esbuild/releases) - [Changelog](https://github.com/evanw/esbuild/blob/main/CHANGELOG.md) - [Commits](evanw/esbuild@v0.25.0...v0.25.1) --- updated-dependencies: - dependency-name: esbuild dependency-type: direct:development update-type: version-update:semver-patch dependency-group: esbuild - dependency-name: esbuild-wasm dependency-type: direct:development update-type: version-update:semver-patch dependency-group: esbuild ... Signed-off-by: dependabot[bot] <[email protected]> Pin novnc to 1.5.0 due to novnc/noVNC#1943
It works for me with ES Modules. A PR has been opened that changes the published NPM Package to use ES Modules instead of shipping invalid CommonJS |
Awesome! |
Thanks @elikoga this will save me a lot of hassle! :-) |
Thanks for working on that! I have same bundling problem and build from @elikoga's branch allowed to bundle noVNC successfully. |
I'll get back to updating the branch with current review changes soon |
Describe the bug
3677afe introduced a top-level await in
lib/util/browser.js
which prevents the library from being bundled with esbuild when previously it was fine.To reproduce
Steps to reproduce the behavior:
npm install -S @novnc/novnc esbuild
index.js
withimport RFB from "@novnc/novnc";
npx esbuild index.js --bundle
Expected behavior
The file bundles successfully.
Server (please complete the following information):
Additional context
This happens even when using esbuild's default target of
esnext
. Is it possible to compile a version ofbrowser.js
that does not use a top-level await?The text was updated successfully, but these errors were encountered: