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

Problems with emcc v3.1.31 in combination with -sMODULARIZE -sEXPORT_ES6 #3238

Closed
WolfgangDrescher opened this issue Jan 28, 2023 · 5 comments

Comments

@WolfgangDrescher
Copy link
Contributor

WolfgangDrescher commented Jan 28, 2023

Currently buildToolkit is not compatible with emcc v3.1.31 when running with -s MODULARIZE=1 -s EXPORT_ES6=1.

The JS build will contain "use strict"; inside the async return function of createVerovioModule():

var createVerovioModule = (() => {
  var _scriptDir = import.meta.url;
  
  return (
async function(config) {
  var createVerovioModule = config || {};

"use strict";var Module=...

This will run into:

Illegal 'use strict' directive in function with non-simple parameter list (Note that you need plugins to import files that are not JavaScript)

I will check if we need additional flags such as -sSTRICT or -sSTRICT_JS or if we need to wait for the next emcc release.

There seems to be already a merged pull request related to this: emscripten-core/emscripten#18614. As well as related issues: emscripten-core/emscripten#18610, emscripten-core/emscripten#18620.

@lpugin
Copy link
Contributor

lpugin commented Jan 30, 2023

Thanks. Hopefully this will be fixed when we make the next release. If not we can build it with an earlier version that still works.

@WolfgangDrescher
Copy link
Contributor Author

WolfgangDrescher commented Jan 30, 2023

I was able to compile it with -s STRICT_JS=0. This makes sense as ES module are always "use strict"; anyways. But the current fix in emscripten will remove -s STRICT_JS=1 as default value when MODULARIZE or EXPORT_ES6 is set. So we should be fine with the next emcc release.

When this is not released yet for the next verovio release I can create a small PR: https://github.com/WolfgangDrescher/verovio/tree/fix-3238

Or we can implement something like this:

https://github.com/mackyle/sqlite/blob/a5de80effaf1e60d110e894a28bdf4144aff5311/ext/wasm/GNUmakefile#L527-L533

If not we can build it with an earlier version that still works.

It looks like older version had the same problem but I only noticed it now because a dependency is more strict with strict mode (either Rollup.js oder Vite, but I'm not sure about this yet).

@lpugin
Copy link
Contributor

lpugin commented Mar 7, 2023

@WolfgangDrescher any update on this?

@WolfgangDrescher
Copy link
Contributor Author

WolfgangDrescher commented Mar 14, 2023

Verovio v3.15.0 was most likely build with a recent version of emcc where the linked PRs are already merged. verovio-module.mjs now looks like this:

var createVerovioModule = (() => {
  var _scriptDir = import.meta.url;
  
  return (
async function(createVerovioModule = {})  {

var Module=...

This looks good now, but I did not test it again. I will close this issue and reopen if I run into this problem again.

@lpugin
Copy link
Contributor

lpugin commented Mar 14, 2023

Great! Many thanks for keeping an eye on this. It is all very helpful.

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

No branches or pull requests

2 participants