RFC: npm package improvements

[WolfgangDrescher]

I have recently resumed the development of the Web Worker integration I started in #1315 (also see #1253). I have come to a point where it would be best if the package code (which is currently just a single file) was split into multiple files so that they could be imported into the worker file without any code repetitions. For this reason, I started to rethink the logic of npm packages for Verovio and came across some questions:

JavaScript build matrix

Why does buildToolkit build multiple versions of Verovio? What is the difference between the JS build and the npm build, except that the npm build exports a node module? What's the difference between the wasm and the non-wasm build? What is the difference between the light and the non-light version. Why do we need so many different versions and what are the use cases? The toolkit matrix in the GitHub workflow is quite big: https://github.com/rism-digital/verovio/blob/develop/.github/workflows/ci_build.yml#L257-L272. It looks like a wasm build with Humdrum support is missing in the ci_build.yml workflow. E.g. like this:

    strategy:
      matrix:
        toolkit:
          - target: nohumdrum
            message: "Building toolkit without humdrum"
            options: "-c -H -M"
            filepath: "verovio-toolkit.js*"
          - target: light
            message: "Building toolkit without humdrum as light version"
            options: "-c -H -l -M"
            filepath: "verovio-toolkit-light.js*"
          - target: wasm-nohumdrum
            message: "Building toolkit without humdrum as wasm"
            options: "-c -H -w -M"
            filepath: "verovio*wasm*"
          - target: wasm
            message: "Building toolkit as wasm"
            options: "-c -w -M"
            filepath: "verovio-toolkit-wasm-hum.js*"
          - target: default
            message: "Building default toolkit with humdrum"
            options: "-c -M"
            filepath: "*-hum.js*"

Bundler

If we decide to split the source code of the current index.js into multiple files, it would probably be a good idea to bundle the code again with a bundler like Webpack or rollup.js. What could speak against this? A bundler could also make sure that the file can be used in multiple environments: node with CommonJS, node with ESM or even directly in the browser. There is a nice example in the Vite documentation that shows how the files could be bundled. See this example for the package.json config:

{
  "name": "my-lib",
  "files": ["dist"],
  "main": "./dist/my-lib.umd.js",
  "module": "./dist/my-lib.es.js",
  "exports": {
    ".": {
      "import": "./dist/my-lib.es.js",
      "require": "./dist/my-lib.umd.js"
    }
  }
}

This could drastically increase the size of the npm package, since the Emscripten build would have to be included once as a source file and a second time inside the final bundled file. However, this could be solved with the files entry in package.json or a .npmignore file: https://docs.npmjs.com/cli/v7/configuring-npm/package-json#files.

In my second example in the next ESM section the Emscripten build would not be bundled, so the size of the bundle would not be a problem.

ESM

It would probably be a good idea to support ESM for the npm packages, as suggested in #2473. That would mean to bump engines.node in package.json to >= 12.0.0. But since Node.js v18 was released just some days ago, the current LTS version is v16, and v12 is only supported until 2022-04-30 I don't think this would be a problem. See https://nodejs.org/en/about/releases/.

ESM would even allow native integration directly in modern browsers, as pointed out in #2473 (https://caniuse.com/es6-module), and it is also possible to use the import and export syntax within a Node.js environment. This can be achieved by using the .mjs file extensions for single files, or by using "type": "module" in the package.json file to enable ESM for the entire package.

I found a configuration for the Emscripten build setp that should work: -s MODULARIZE=1 -s EXPORT_ES6=1. However, this seems to break the current setup for using the Verovio Toolkit in JavaScript, since with this flags the module needs to be created first and then returns a promise that needs to be awaited. But with the promise this seems to be a very straight forward way to set the module up! See https://stackoverflow.com/a/53384917/9009012. It could also solve some problems I had in hot reload mode with the onRuntimeInitialized event.

Something like this could be the new setup script:

import { VerovioWasmFactory, VerovioToolkit } from 'verovio';

VerovioWasmFactory().then(function(VerovioModule) {
    const verovioToolkit = new VerovioToolkit(VerovioModule);
    console.log(verovioToolkit.getVersion());
    verovioToolkit.loadData(score);
    const data = verovioToolkit.renderToSVG(1, {})
});

Another option could be like so:

import VerovioWasmFactory from 'verovio/wasm/verovio-toolkit-wasm-hum.js';
import { VerovioToolkit } from 'verovio';

VerovioWasmFactory().then(function(VerovioModule) {
    const verovioToolkit = new VerovioToolkit(VerovioModule);
    console.log(verovioToolkit.getVersion());
    verovioToolkit.loadData(score);
    const data = verovioToolkit.renderToSVG(1, {})
});

Or directly in a modern browser:

<script type="module">
    import VerovioWasmFactory from './verovio/wasm/verovio-toolkit-wasm-hum.js';
    import { VerovioToolkit } from './verovio/index.mjs';

    VerovioWasmFactory().then((VerovioModule) => {
        const verovioToolkit = new VerovioToolkit(VerovioModule);
        console.log(verovioToolkit.getVersion());
        verovioToolkit.loadData(score);
        const data = verovioToolkit.renderToSVG(1, {});
    });
</script>

I think the second option would be the best solution, although the import is a bit verbose with two statements. I have decoupled the dependency from a global Module variable and the toolkit now has to get the Module instance passed in the constructor. This would allow the toolkit to be bundled with a tool like rollup.js or Webpack, but the Emscripten build itself would not be included in that build since you have to import it separately. This would even allow multiple versions of the JS build (e.g. the current build matrix from the ci_build.yml pipeline) to be delivered within the same npm package, so the new verovio-humdrum npm package would no longer be needed. All built variants can be added into the wasm folder. This would only increase the file size of the verovio npm package, but that doesn't seem to become problematic for 20MB or so: npm/npm#12750 (comment). The bundle size of the developer application wouldn't explode either, since only the Module version they import will be bundled.

In any case, this would be a breaking change and therefore it would be a good idea to implement it with the next major release v4.0.0.

Web Worker

I've been researching Web Worker integration for npm packages and I think the best solution would be for Verovio to ship a file that developers can use to let their bundlers build the worker file for them.

The best example I found is from the Monaco Editor repository (Visual Studio Code). See https://github.com/microsoft/monaco-editor/blob/main/docs/integrate-esm.md and vitejs/vite#1791 (comment).

The Vite example (it is using rollup.js behind the scenes) for integrating the ESM version of the Monaco editor as a web worker is easy to understand. Vite can setup a file as a web worker by simply adding ?worker after the import statement. We should leave it to the verovio package users to create the worker, but provide a file from which they can generate it. E.g.:

import VerovioWorker from 'verovio/verovio.worker?worker';

window.VerovioEnvironment = {
    createWorker() {
         return new VerovioWorker();
    },
};

Insinde the verovio package we could check if the VerovioEnvironment.createWorker variable is present, if so create the toolkit as a worker and if not with native JavaScript:

createToolkitProxy() {
    return self.VerovioEnvironment?.createWorker ? this.createWorkerProxy() : this.createNativeProxy();
}

That was initially why I wanted to refactor the vervio npm package so that the 10MB Emscripts wasm build doesn't have to be included twice in the package, but it can be imported.

Without Vite or with another bundler, as far as I know, a worker can be created as follows: new Worker(new URL('./file/to/verovio.worker.js', import.meta.url)));. But I haven't tested this myself yet.

Since the worker must use async method calls on the toolkit (see https://github.com/WolfgangDrescher/verovio-worker), I suggest also adding a Toolkit Proxy (maybe with the adapter pattern) for the non-worker version to execute Verovio toolkit calls fake asynchronously (the promise becomes resolved immediately) to make the syntax is interchangeable, no matter whether you use a worker or not. I would suggest something like that:

Click to expand the code example

class Deferred {
    constructor() {
        this.promise = new Promise((resolve, reject) => {
            this.reject = reject;
            this.resolve = resolve;
        });
    }
}
createWorkerProxy() {
    this.tasks = {};
    this.worker = self.VerovioEnvironment.createWorker();
    this.worker.addEventListener('message', (event) => {
        const { id, result } = event.data;
        const deferred = this.tasks[id];
        if (deferred) {
            deferred.resolve(result);
            delete this.tasks[id];
        }
    });
    this.worker.addEventListener('error', (event) => {
        console.error(event);
    });
    return new Proxy(this, {
        get: (target, method) => {
            return (...args) => {
                const id = uuidv4();
                target.worker.postMessage({
                    id,
                    method,
                    args,
                });
                const deferred = new Deferred();
                this.tasks[id] = deferred;
                return deferred.promise;
            };
        },
    });
}
createNativeProxy() {
    moduleIsReady.promise.then(() => {
        this.verovioToolkit = new verovio.toolkit();
    });
    return new Proxy(this, {
        get: (target, method) => {
            return (...args) => {
                const deferred = new Deferred();
                if(method === 'onRuntimeInitialized') {
                    moduleIsReady.promise.then(() => {
                        deferred.resolve();
                    });
                } else {
                    const fn = this.verovioToolkit[method || null];
                    if(fn) {
                        deferred.resolve(fn.apply(this.verovioToolkit, args || []));
                    }
                }
                return deferred.promise;
            };
        },
    });
}

window.onunload event

Is the window.onunload event that destroys the wasm instance really needed? As far as I know, the browser does this itself. MDN also warns against using this event: https://developer.mozilla.org/en-US/docs/web/api/window/unload_event.

---

I have set up a repository to showcase some of these features: https://github.com/WolfgangDrescher/verovio-npm-improvements. It's still work in progress and not very mature yet, but for a first preview it can give a good insight I think. To try it out you can add "verovio": "github:WolfgangDrescher/verovio-npm-improvements", into the package.json dependencies. I can also provide a small example app in Vue.js if there is interest.

[WolfgangDrescher]

I just saw that this is also related to: #1908. Of course I can open a pull request, but these are major and breaking changes, so I think it's better to discuss about the integration first.

Also note that my demo (https://github.com/WolfgangDrescher/verovio-npm-improvements) seems to break currently. I get a JS error RuntimeError: memory access out of bounds that gets thrown in _vrvToolkit_loadData. First I thought that my wasm file is buggy since I got some errors during the Emscripten build. But then I built the toolkit with a GitHub workflow from ci_build.yml and got the same result. Has this already occurred somewhere else? Or did the new set up somehow messing this up?

[lpugin]

A few comments on this initial post.

List of builds

Why does buildToolkit build multiple versions of Verovio?

The different builds have historical reasons. WebAssembly did not exist when we started to use Emscripten, this is why we have a plain JS (asm.js) build and a wasm build. We needed to add a -light version because we wanted to make it usable on machines with reduced resources. We also agreed to have the humdrum toolkit a distinct build because not all users need it and the size difference between the two builds is not insignificant.

We can certainly re-think what we have in the CI and I agree we can maybe reduce the list. I would suggest to remove the -light build from the CI and keep it only for the releases.

What's the difference between the wasm and the non-wasm build?

The non-wasm build is asm.js, which we the original output of Emscripten when WebAssembly did not exist. We have both because for a long time many browsers did not support wasm. Also it is simpler to include in a web page, and many projects still use it. Not that I would recommend it because the loading time can be quite long, but I don't think we can drop it.

What is the difference between the JS build and the npm build, except that the npm build exports a node module?

Basically none - assuming you mean the JS wasm build. We have the npm build only for releases, and I think this is fine. We do the same with PyPi.

Why do we need so many different versions and what are the use cases?

I hope the points above answer the question, please let me know if not.

I understand we need to make changes to the current setup to accommodate ES6 uses. Only a few remarks on the changes:

• As you can see, testing these things can be very tricky, so you better do changes step by step so we can test them. (I'll try to see what the memory issue is in your repo.)
• Users run it from node, so this is definitely something we want to maintain and it should remain simple.
• I am not sure why we would need to adopt a bundler for a single package. I might be overlooking something, but it would be preferable to stay away from an additional dependency if it is not strictly needed.

[WolfgangDrescher]

I see, thank you for the explanation. Im not very deep into Emscripten and Wasm, so please just correct me if I'm misunderstanding something.

I created another testing repository on how to implement all different JS versions of verovio: https://github.com/WolfgangDrescher/verovio-js-examples. This is for browser usage without Node.js, ESM script tag modules or a bundler. Note that Module.onRuntimeInitialized is actually not working for non Wasm file (see this commit). I'm not really sure if this is intended, since you can find if(Module["onRuntimeInitialized"])Module["onRuntimeInitialized"](); in all four JS builds. If this is intended I don't think that this is documented somewhere?

So in total we have three ways of how to integrate the Verovio JS toolkit:

1. plain JavaScript files for direct browser support (see here)
2. direct Node.js usage via npm and interacting with the file system like shown in the npm README.md
3. importing it with npm but bundling it into a web app with Vue.js or React, etc. like e.g. with my vue plugin for Verovio

In an ideal scenario there would be just one JS file that will work for all of those purposes, right (maybe two when keeping humdrum separately)? But probably this will not work. For example ESM is working in all browsers, but only when served with a web server and not with statically served HTML files like my examples from point 1. would work.

Or do you prefer to keep the plain JS files (verovio-toolkit-hum.js, verovio-toolkit-light.js, verovio-toolkit-wasm.js, verovio-toolkit.js) untouched and only modify the npm package?

So it will be important to specify the exact purpose of the npm package. Does it only have to work in a Node context or when bundled into a web app, or could the npm version of Verovio just be a general JS build that ideally will work for all contexts, or at least provide a build for every needed context.

As you can see, testing these things can be very tricky, so you better do changes step by step so we can test them. (I'll try to see what the memory issue is in your repo.)

Actually I'm surprised how fast things got complicated. Once we have all requirements together I can try to split them up into multiple smaller tasks.

Users run it from node, so this is definitely something we want to maintain and it should remain simple.

Okay, I will have to do a lot of testing to keep everything compatible and still provide a modern ESM version. Although emscripten-core/emscripten#11792 concerns me a bit because it look like Emscripten does not fully support ESM for node but only with bundlers (browser context). The Module still depends on require("path"), require("fs"), require("crypto") and __dirname, which are all built in packaged into node, but ESM modules in node are not able to import them like this.

I am not sure why we would need to adopt a bundler for a single package. I might be overlooking something, but it would be preferable to stay away from an additional dependency if it is not strictly needed.

The bundler would just be a helper to build multiple versions of the package. E.g. a bundler could handle CJS and ESM builds without the need for us of maintaining multiple version of verovio-npm-start.js and verovio-npm-end.js for each environment. The bundler can also allow us to use modern JS syntax in the source code, like the spread operator ...args used in my demo here but kind of compiles it to code that will be backwards compatible with older browsers that do not support those features. Bundlers usually also allow to chose a context (node or browser, and you can even specify the supported browser). This will allow us to split code up into multiple files which is easier to maintain and the bundler will build a single file out if it. It will replace cat verovio-npm-start.js $BUILD_DIR/verovio.js verovio-proxy.js verovio-npm-end.js > $NPM/index.js in buildToolkit.

Bundler usually also minify the code to save some bytes. However for this I think it's better if only the JS gets bundled and the Wasm file will not be touched again by the bundler. This is why I separated them in verovio-npm-improvements. The bundler will only touch files inside ~/src like in my example, but it will not touch files inside ~/wasm.

Also note, that I added the bundler as a devDependency inside package.json so it will actually not be included in the built package but only be a dependency needed to build a dist version of the package. This will keep the verovio package itself completely dependency free. Think of it as a build step for JS.

Depending on the requirements it could be a good idea to split up the Wasm module and the Verovio toolkit wrapper to keep them separately and including humdrum and non-humdrum in the same package like I did in verovio-npm-improvements. If not, the build itself could also contain the wasm Module itself.

[lpugin]

Note that Module.onRuntimeInitialized is actually not working for non Wasm file (see this commit). I'm not really sure if this is intended, since you can find if(Module["onRuntimeInitialized"])Module["onRuntimeInitialized"](); in all four JS builds. If this is intended I don't think that this is documented somewhere?

The recommended way to use the JS toolkit is documented here https://book.verovio.org/first-steps/getting-started.html

The verovio-toolkit.js and verovio-toolkit-light.js as mostly there for backward compatibility for projects that still use them. We should not recommend to use them. You are right that onRuntimeInitialized is not defined there because the loading happens synchronously.

Or do you prefer to keep the plain JS files (verovio-toolkit-hum.js, verovio-toolkit-light.js, verovio-toolkit-wasm.js, verovio-toolkit.js) untouched and only modify the npm package?

I don't see the need to change anything in verovio-toolkit.js and verovio-toolkit-light.js for the same reasons as above.

Once you have something working we can see what should be change or not, but as this stage it seems difficult to decide. If we change the way the verovio-toolkit-wasm.js works, then we need to evaluate how much it will break of our current environments. That means that, yes, I would prefer verovio-toolkit-wasm.js to remain untouched.

Thank you for the explanation on the bundler. If we use it only for generating the package and it is not a dependency to use it then it should be fine.

[WolfgangDrescher]

Once you have something working we can see what should be change or not, but as this stage it seems difficult to decide. If we change the way the verovio-toolkit-wasm.js works, then we need to evaluate how much it will break of our current environments. That means that, yes, I would prefer verovio-toolkit-wasm.js to remain untouched.

I just realized that I missed to add toString() in this line fs.readFileSync('demo.krn').toString() in my examples. Sadly things are still bugged:

$ cd node-esm
$ node index.mjs
Creating toolkit instance
3.10.0-dev-8aee76f
[1]    2669 bus error  node index.mjs

And in the browser example there is still RuntimeError: memory access out of bounds. Do you experience the same error when you pass these additional flags -s MODULARIZE=1 -s EXPORT_ES6=1 to ./buildToolkit?

Ah or maybe I should trigger a build from tag version 3.9.0 with these flags instead of the current commit in the develop branch?

[lpugin]

Thank you for looking into this. Let's move it to a discussion and then have issues for each topic separately whenever needed.

[WolfgangDrescher]

Ah sorry, I missed that GitHub discussions are open now.

[WolfgangDrescher]

After further research, the decision for ESM support, or rather the way in which it should be implemented, should be made carefully. Further thoughts and options on this topic:

1. Switch completely to ESM with "type": "module" in package.json.
2. Provide multiple entry points with "main": "dist/cjs/index.[c?]js" and "module": "dist/mjs/index.[m?]js". Or as in the example from the Vite documentation of my original post.
3. Explicitly specify exports with "exports": {".": {"require": "./index.[c?]js", "import": "./index.[m?]js"}} in package.json.
4. How to provide CommonJS and ESM versions of the Emscripten modules without having to have the wasm files multiple times in the npm package? Maybe the wrapper pattern could be useful for this? (See here and here)

From my understanding deciding for ESM should also be a decision for a bundler that handles all the different versions for the verovio npm package. esbuild looks like an interesting tool. But Rollup.js, Webpack or another bundler should also do the trick.

Some sources:

• https://antfu.me/posts/publish-esm-and-cjs
• https://github.com/FormidableLabs/node-esm-examples
• https://formidable.com/blog/2021/node-esm-and-exports/
• https://gist.github.com/sindresorhus/a39789f98801d908bbc7ff3ecc99d99c
• https://www.sensedeep.com/blog/posts/2021/how-to-create-single-source-npm-module.html

[WolfgangDrescher]

I have set up a repository to test some functions with different ways to include Verovio: https://github.com/WolfgangDrescher/verovio-npm-examples. Implementing this and with further research, I found out a bunch of interesting things.

---

1. The module field in package.json is actually not officially supported by Node.js but used to be used by some bundlers (see here). So using the exports field is important to expose all exports of the package.

---

2. Turning on EXPORT_ES6 in the Emscripten build script will actually break support to execute the Verovio toolkit directly in node in the CLI since the the built file will actually still include __dirname. There is an open issue for this in the emscripten repository since august 2020: EXPORT_ES6: Add testing and support for node.js (replace require(), __dirname) emscripten-core/emscripten#11792. However there is a workaround for this:

import {dirname} from 'path';
globalThis.__dirname = dirname(import.meta.url);
import Module from 'verovio/wasm/verovio-toolkit-wasm-hum.js';

This fix will work if you run the node script with ESM (e.g. the .mjs file extension; see node-esm demo). However I don't think that this is problematic, since users that can handle node would probably prefer using the Verovio CLI rather than executing the toolkit with node.

Somehow running this Emscripten module with frameworks like Vue.js or React their bundlers don't seems to bother about this.

---

3. Running the EXPORT_ES6 build of the Emsciprten module in CommonJS of course won't work since ES modules cannot be imported by CommonJS with require(). But there is also a workaround for this by using the async import method:

const { VerovioToolkit } = require('verovio');
const fs = require('fs');

import('verovio/wasm/verovio-toolkit-wasm-hum.js').then(Module => {
    console.log(Module.default);
    Module.default().then((VerovioModule) => {
        const verovioToolkit = new VerovioToolkit(VerovioModule);
        console.log(verovioToolkit.getVersion());
        const score = fs.readFileSync('demo.krn');
        verovioToolkit.loadData(score);
        const data = verovioToolkit.renderToSVG(1, {});
        console.log(data);
    });
});

But this again will lead to the __dirname is not defined problematic since also here in the ESM build of Emscripten there is __dirname present. See the node-commonjs demo.

---

4. When using the CommonJS build of the Emscripten module (EXPORT_ES6=0) of course the browser integration will not work anymore with <script type="module"> since the imported wasm module is not actually exporting an ES module. Using a UMD build could solve this for the browser and still allow CommonJS support, but would not be compatible with native ESM. Although there could potentially be a solution for this with the wrapper pattern. I did not test if the paths of the imports with the wrapper pattern actually work in browser script modules. Worst case would be that the Emscripten builds would have to be shipped with both versions: ESM and CJS.

---

5. Using the CommonJS build of the Emscripten module (EXPORT_ES6=0) with the node-esm demo (checkout the cjs branch) will actually work as long as you just import the default scope. (see "Importing CommonJS in ESM" and here). So maybe it could be a solution to use the CJS export of Emscripten with UMD to also support the module in global scope in the browser but use ESM for the the rest of the verovio package.

Direct browser support could look like this:

<script type="module" src="verovio/wasm/verovio-toolkit-wasm-hum.js"></script> <!-- this exposes some kind of global variable for the Module like window.VerovioWasmFactory -->
<script type="module">
    import { VerovioToolkit } from './verovio/index.mjs';

    VerovioWasmFactory().then((VerovioModule) => {
        const verovioToolkit = new VerovioToolkit(VerovioModule);
        console.log(verovioToolkit.getVersion());
        verovioToolkit.loadData(score);
        const data = verovioToolkit.renderToSVG(1, {});
    });
</script>

---

6. The react-app demo complains about other missing modules when using the CJS build. To me it looks like Emscripten is not quite perfect for browser usage, or needs an additional build step for this.

Click to expand logs

Failed to compile.

Module not found: Error: Can't resolve 'path' in '~/verovio-npm-examples/react-app/node_modules/verovio/wasm'
BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
	- add a fallback 'resolve.fallback: { "path": require.resolve("path-browserify") }'
	- install 'path-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
	resolve.fallback: { "path": false }
WARNING in ./node_modules/verovio/wasm/verovio-toolkit-wasm-hum.js 982:30-47
Module not found: Error: Can't resolve 'crypto' in '~/verovio-npm-examples/react-app/node_modules/verovio/wasm'

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
	- add a fallback 'resolve.fallback: { "crypto": require.resolve("crypto-browserify") }'
	- install 'crypto-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
	resolve.fallback: { "crypto": false }

ERROR in ./node_modules/verovio/wasm/verovio-toolkit-wasm-hum.js 92:26-49
Module not found: Error: Can't resolve 'path' in '~/verovio-npm-examples/react-app/node_modules/verovio/wasm'

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
	- add a fallback 'resolve.fallback: { "path": require.resolve("path-browserify") }'
	- install 'path-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
	resolve.fallback: { "path": false }

ERROR in ./node_modules/verovio/wasm/verovio-toolkit-wasm-hum.js 99:15-28
Module not found: Error: Can't resolve 'fs' in '~/verovio-npm-examples/react-app/node_modules/verovio/wasm'

webpack compiled with 2 errors and 1 warning

---

Please note that all demos currently fail because of the already mentioned RuntimeError: memory access out of bounds error I get. I still need to figure out the problem here. Potentially bad options and flag passed to the emscripten build step.

[ahankinson]

How are you building the Verovio toolkit? Are you using the light build option? Note that this primarily controls memory pre-allocation, so if you're running into memory errors you can try omitting the -l flag and see if that fixes it.

[WolfgangDrescher]

Checkout the two lastest GitHub actions on https://github.com/WolfgangDrescher/verovio/actions/workflows/ci_build.yml. It should actually not use the light build option.

https://github.com/WolfgangDrescher/verovio/runs/6145452266

./buildToolkit -c -w -M

https://github.com/WolfgangDrescher/verovio/runs/6145452238

./buildToolkit -c -H -w -M

These are the two files I added to https://github.com/WolfgangDrescher/verovio-npm-improvements/tree/master/wasm.

Since I did not trust the build I created locally because some errors did show up, I decided to let GitHub build the toolkit with the official workflow. After the build I had to modify the file slightly and strip out all the custom JavaScript that buildToolkit adds to the Emscripten build. I basically just removed the content of ~/emscripten/verovio-npm-start.js, ~/emscripten/verovio-proxy.js and ~/emscripten/verovio-npm-end.js from the file.

[ahankinson]

Would it be possible to define a "Proxy module" for an ES6 environment that abstracted away the need to make changes to the Verovio build directly? I'm thinking that you could define a module (verovio-module.mjs or something?) that would export the necessary functions, and maybe even load the core Verovio library as a worker.

Then you could interact with the proxy in an ES6 environment, but the underlying library does not need to change.

[WolfgangDrescher]

Good idea. I did setup a quick demo in https://github.com/WolfgangDrescher/verovio-npm-esm-proxy with the already mentioned wrapper pattern and pushed a new esm-proxy branch in my verovio-npm-exampels repository. This is the result of the test examples:

1. Direct browser integration does fail because of module is not defined. This can be solved if we would use a UMD build. It would allow the JS file to work in the browser but also within node. This would also remove the need for files like verovio-npm-end.js and verovio-js-start.js etc. but will need a build step with a bundler.
2. Node with CJS: works
3. Node with ESM: works
4. Vue app: works
5. React app: fails with Error: Can't resolve 'path' in '~/verovio-npm-examples/react-app/node_modules/verovio'. But this seems like an issue with webpack 5 and seems to be related to the imported native dependencies of the Emscripten build I already mentioned.

However I don't think we actually have a benefit from this, because with the current npm package it is also possible to just import CommonJS modules into ESM. In fact I think there is not a big difference if user of the package imports the CJS module into this ESM setup, or if we support a ESM exported version of the CJS module. I think in my projects with Vue and/or Vite this is already happening with the current npm package (Vite actually transpiles it). E.g. there would still not be support for tree shaking without a native ESM build.

At the moment I'm not sure anymore if it is actually a good idea to migrate the project to support ESM for now. Many breaking changed would be needed and the Emscripten build of the Module does not seem bulletproof yet for ESM in native Node.

Maybe a hybrid solution could work. Where the Emscripten build will be a .cjs file and the rest of the package could be native ESM. Still this would need breaking changes to separate this into multiple files.

Is there a roadmap for verovio? When is the next major release planned?

Here is a list of features and changes to sum up my ideas from this discussion:

1. ESM support with -s EXPORT_ES6=1 when there is a working solution for all environments: Node, Browser, bundled JS Apps (breaking change)
2. Split up the code into multiple files or easier maintenance and future features (Web Worker) and bundle them again into a dist build with a bundler
3. Promise based Module setup with -s MODULARIZE=1 for easier integration (breaking change)
4. Keep the Emscripten build in a separate file or even directory with an additional import statement to setup verovio to enable multiple versions of the Module in one package, e.g. with humdrum and without (tbd, breaking change)
5. A Web Worker template

[lpugin]

Is there a roadmap for verovio? When is the next major release planned?

As given in the readme.md, even number major releases for Verovio are planned to be inline with MEI releases. In other words, the next major release will be made when MEI 5.0.0 is released.

[lpugin]

One project you can look at is https://github.com/MTG/essentia.js

[WolfgangDrescher]

This is a very interesting project and setup that they use at essentia.js, thank you for the link! Actually this is quite similar to my suggestions in: verovio-npm-improvements

Some notes on the setup of this project:

1. They use a separate repository for the js/npm build of Essentia. I think this could also be a good idea for verovio. This could also be integrated as a submodule into the main repository in the emscripten folder.
2. I like the idea to add examples directly into this repository. They will not be shipped in the npm package itself (due to the files filed in package.json) but still they are around for documentation purposes.
3. Their source code is in TypeScript (~/essentia.js/src/typescript/core_api.ts) and will be bundled into plain JS in the dist folder.
4. They use rollup.js to bundle everything together and build multiple versions: ESM, UMD, IIFE and minified alternatives for all of these options (see build options here and here). This of course means, that the ~10MB module for verovio would have to be included multiple times in the npm package.
5. They also separate out the wasm file from the rest of the code. So two imports are needed. Also the wasm module needs to be passed to the constructor new Essentia(EssentiaWASM).
6. Note that they have an async build for the web and a synchronous build for node usage. I'm not sure why they make that difference. Checkout their Makefile here. -s ENVIRONMENT=web -s MODULARIZE=1 is used for the web build. I'm also not sure what the essentia-wasm.web.wasm file is used for.

Here is a screenshot of the directory structure of the essentia.js package. As you can see only the dist folder will be included:

If you have a look at their index.js you can see that they export the module similar to the current verovio export. Maybe we could have a transition period where we still support the current method how to implement verovio, but at the same time introduce a new way. But this will probably mean to include different builds of the emscriptem wasm file intro the package.

[lpugin]

1. They use a separate repository for the js/npm build of Essentia. I think this could also be a good idea for verovio. This could also be integrated as a submodule into the main repository in the emscripten folder.

I am not a big fan of submodules. Because the JS build part is pretty small it is OK to have it in the main repository in my opinion.

Maybe we could have a transition period where we still support the current method how to implement verovio, but at the same time introduce a new way.

Yes, that is a good idea.

[WolfgangDrescher]

I'll try to see what the memory issue is in your repo.

I managed to fix the issue with memory access out of bounds. It was actually a bug on my side (I passed an array of arguments to cwrap and missed to use the spread operator). So the emscripten builds should be fine and were never broken.

I updated https://github.com/WolfgangDrescher/verovio-npm-examples. On the master branch the node-commonjs example is the only one that is not working. On the cjs branch obviously the browser-script-module example is not working and react-app somewhat works but still throws webpack errors. All in all much better now!

Now I can experiment further with a backward compatible package. Some new thoughts:

• I switched to rollup.js instead of esbuild for now, since esbuild is currently not supporting UMD (Introduce UMD as a new output format evanw/esbuild#1331)
• UMD has the advantage that it is possible to run the package in the browser but also in node. The problem is, that rollup.js can only expose a single variable to export or assign to window. So in browser context the window.Module will not be exposed. Instead window.verovio.module can be used. This is good, but agin a breaking change. If this is a needed requirement to keep the Module variable on the global scope I can put more effort into this. But I'm not sure if it is possible when using bundlers.
• All JS builds should be created with the new codebase so we don't have to maintain multiple versions at the same time. The built versions of this should be compatible with the browser and node. This means that buildToolkit will just create the plain emscripten modules as a JS file (and add some kind of export at the end with file concatenation). Then we can replace cat verovio-npm-start.js $BUILD_DIR/verovio.js verovio-proxy.js verovio-npm-end.js > $NPM/index.js with npm run build in the buildToolkit command.
• This also means that the GitHub workflow needs to be refactored to create the artefacts.
• At the end the new setup will create the already known files verovio-toolkit.js, verovio-toolkit-wasm.js, verovio-toolkit-light.js, verovio-toolkit-hum.js and a new verovio-toolkit-wasm-hum.js but also include a new way how to setup verovio with ESM. Only verovio-toolkit-wasm.js and/or verovio-toolkit-wasm-hum.js will be included in the npm package (again with the files filed in package.json) as a fallback (comparable the the index.js of the current npm package) and the new ESM compatible setup.

Anything I need go consider before I continue with the development?

I am not a big fan of submodules. Because the JS build part is pretty small it is OK to have it in the main repository in my opinion.

Okay, I also try to avoid submodules whenever possible. Still I think having a dedicated repository for the npm package would be easier for beginners to get started with verovio so they don't need to understand the full verovio repo structure with emscripten, c++ and all if they just want to use the JS build. And it could be better to find examples in the repo with a good README.md in the npm package repo. Maybe git subtree split could be another option? The symfony project could be a nice inspiration for this. It is a monolith that splits into multiple read-only repositories:

Fabien Potencier, the creator fo Symfony, has a talk on YouTube about this: https://youtu.be/4w3-f6Xhvu8. In fact they are using split.sh, written in Go and libgit2, that is much faster than git subtree split. But this is probably an overkill for Verovio.

[lpugin]

I am confused. onRuntimeInitialized is not to be called when using the verovio-toolkit.js (and the verovio-toolkit-light.js), so even if the name of the module is changed internally, I do not see why it would break. But I am probably overlooking something.

I will update my repository to make this clear.

Yes, that will be helpful.

For compatibility and to stay consistent personally I would keep the verovio-toolkit-hum-wasm.js naming for now.

Sounds good to me.

Is there any reason why it could be better to use a non-wasm build?

Except for the various reasons we already discussed, not really.

[WolfgangDrescher]

I am confused. onRuntimeInitialized is not to be called when using the verovio-toolkit.js (and the verovio-toolkit-light.js), so even if the name of the module is changed internally, I do not see why it would break. But I am probably overlooking something.

Ah sorry, you are right. I missed that verovio-toolkit.js and verovio-toolkit-light.js are synchronous. For these two it should be possible to add a fallback when no module is passed to the constructor. They should have access to the local Module variable in the bundle:

export class VerovioToolkit {

    constructor(VerovioModule) {
        this.VerovioModule = VerovioModule || (typeof Module !== 'undefined' ? Module : null);
        if(!this.VerovioModule) {
            throw new Error('VerovioToolkit could not find emscripten module.');
        }

    // ...

}

I will test it this works for all environments.

Still, for verovio-toolkit-wasm.js I would have to find a workaround if we want full backwards compatible support.

[lpugin]

Still, for verovio-toolkit-wasm.js I would have to find a workaround if we want full backwards compatible support.

In my opinion it would be fine if compatibility is broken for the wasm toolkit. The reason it that it would actually be better to have a distinct module name because Module can conflict. See #2473 (comment)

[WolfgangDrescher]

In my opinion it would be fine if compatibility is broken for the wasm toolkit.

But once we allow breaking changes, we might be able to solve several problems with it and try to get the perfect setup (e.g. adding MODULARIZE). But I understand that Module.onRuntimeInitialized vs. verovio.module.onRuntimeInitialized is not a huge change. Still breaking. But then this is actually how to initialize verovio in Node.js as shown in the current README.md of the npm package. So it would also be an argument to adapt the browser setup to this.

The reason it that it would actually be better to have a distinct module name because Module can conflict. See #2473 (comment)

Good point. I was also thinking about adding -s EXPORT_NAME="VerovioModule", but just to name it explicit. When the Module is exposed with verovio.module instead of Module directly this should not be necessary, but still it's a good idea in my opinion.

First tests are looking good so far. Almost all examples are working. Only native Node.js is not working yet (CJS and ESM). I managed to pass a default module to the constructor with the following code:

import Module from '../modules/verovio-toolkit-wasm.js'
import { VerovioToolkit } from './verovio-toolkit.js';

class VerovioLegacyToolkit extends VerovioToolkit {
    constructor(VerovioModule = Module) {
        super(VerovioModule);
    }
}

export default {
    module: Module,
    toolkit: VerovioLegacyToolkit,
}

[WolfgangDrescher]

Non-wasm builds (light, hum and verovio-toolkit.js) are expected not to work in a Node.js context, right? As currently only the wasm builds get appended the verovio-npm-end.js file with the module.exports. With the new UMD build of light, hum and verovio-toolkit.js they could theoretically be imported in a node context but I get an error:

TypeError: Cannot read properties of undefined (reading 'z')
    at Module._vrvToolkit_constructor (~/verovio-npm-examples/node-legacy/node_modules/verovio/dist/verovio-toolkit-hum.js:50:78076)

So only wasm builds need to work with node, correct?

[WolfgangDrescher]

I have a version ready that passes all tests and that I'm happy with for now. Check out the v2 branch in https://github.com/WolfgangDrescher/verovio-npm-examples/tree/v2 for the examples and the v2 branch in https://github.com/WolfgangDrescher/verovio-npm-improvements/tree/v2 for the updated package.

Comments and explanation

• The only breaking change is the one we already discussed: when using verovio directly in the browser Module is no longer exposed in global scope but has to be accessed with verovio.module.
• When importing the package with import verovio from 'verovio' you will get a module that is fully functional as the current npm package, but bundled with rollup.
• package.json will expose an export field with a mapping to all needed files:

{
    "main": "dist/verovio-toolkit-wasm.js",
    "exports": {
        ".": "./dist/verovio-toolkit-wasm.js",
        "./next": {
            "import": "./dist/index.mjs",
            "require": "./dist/index.cjs"
        },
        "./wasm": "./modules/verovio-toolkit-wasm-modularize.js"
    }
}

• If you want to use the new ESM import use import { VerovioToolkit } from 'verovio/next'. With this new setup the emscripten module has to be imported separately: import createVerovioModule from 'verovio/wasm'.
• Naming and final directory structure is not perfect and probably has to be discussed.
• We can easily switch import paths verovio/next with verovio to make it default and expose the old import style as verovio/legacy or similar. This would break, if people expect import verovio from 'verovio' to work as it always did.
• verovio-toolkit.js, verovio-toolkit-hum.js, verovio-toolkit-light.js and verovio-toolkit-wasm.js don't not have to be included in the package because out of these only wasm is needed and it is already bundled in the default "legay build" (./dist/verovio-toolkit-wasm.js)
• ./dist/verovio-toolkit-hum.js, ./dist/verovio-toolkit-light.js and ./dist/verovio-toolkit.js don't have to be included in the package. They are just here so we can use them somewhere as GitHub action artifacts for the releases.
• Only verovio-toolkit-wasm-modularize.js is included as a plain emscripten build. It is built with -s MODULARIZE=1 so the new integration will need the factory pattern:

const createVerovioModule = require('verovio/wasm');
const { VerovioToolkit } = require('verovio/next');

createVerovioModule().then(VerovioModule => {
    const verovioToolkit = new VerovioToolkit(VerovioModule);
});

• Best naming for the emscripten module would be createVerovioModule as suggested the emscripten docs. We will have to pass it with -s EXPORT_NAME="createVerovioModule"
• As already discussed when we are already refactoring it will also be a good idea to rename the exported module name (for toolkit, hum, light, wasm) as -s EXPORT_NAME="VerovioModule"
• The legacy emscripten builds (toolkit, hum, light, wasm) will need an additional module.exports = Module; at the end to work properly. Either adding it by hand with echo "module.exports = Module;" >> verovio-toolkit-wasm.js or with --post-js file/to/wasm.module.post.js or with a rollup plugin that I already included: rollup-plugin-add-module-exports.js.
• The browser-script-module example needs a small hack to include the emscripten module since it is CJS for best support. Just import it with a regular script tag and not a script tag of type="module". But for now you can always fallback to the legacy build.

<script src="./node_modules/verovio/modules/verovio-toolkit-wasm-modularize.js" defer></script>
<script type="module">
    import { VerovioToolkit } from './node_modules/verovio/dist/index.mjs';
    // ...
</script>

• Probably we can get rid of the ./modules directory and include the needed emscripten modules directly in ./dist
• The dist directory typically has not to be committed, I did it here for the sake of easier illustration.

Naming that need to be discussed

• Export path for new ESM integration. My suggestion is verovio/next. Later we can make it the default by removing /next.
• Class name for VerovioLegacyToolkit is probably better as VerovioToolkitWithDefaultModule or similar.
• Filenames in ./src folder
• Export path for the emscripten module. For now im happy with verovio/wasm but if we want to include humdrum later into this package it would have to be named differently. As long was we still support the legacy build as default there is still the need to have verovio-humdrum as separate npm package.
• Filename of verovio-toolkit-wasm-modularize.js for the new emscripten build.
• Filename of emscripten-proxy.js, it's actually just a mapping between the toolkit methods and the emscripten module cwrap calls.

[lpugin]

FYI, we do not use the GH to build the release but only for the CI integration. As I mentioned already I would be in favour or reducing the CI matrix instead of extending it. We have often quite a few build per day, and the amount of data generated matters at some point.

The NPM packages are published manually at release time - it is the same for the Python ones. This is not likely to change with the pace of a release every two months or so. So it it really a matter of having the list of script we need to run to build the toolkit and the package locally before we publish them.

[WolfgangDrescher]

Actually it's not possible to publish the same version of a package with another npm dist-tag:

npm ERR! code E403
npm ERR! 403 403 Forbidden - PUT https://registry.npmjs.org/@wolfgangdrescher%2fverovio-dist-tag-test - You cannot publish over the previously published versions: 3.9.0.
npm ERR! 403 In most cases, you or one of your dependencies are requesting
npm ERR! 403 a package version that is forbidden by your security policy, or
npm ERR! 403 on a server you do not have access to.

But when you pass a version tag to the package version number it will kind of work:

{
    "name": "@wolfgangdrescher/verovio-dist-tag-test",
    "version": "3.9.0-next",
    // ...
}

According to https://semver.org/ this should be valid for a npm version number. However this will be treated as a prerelease to 3.9.0 and therefore you will never be able to use the package with "verovio": "^3.9.0-next" because version 3.9.0 is actually a higher version number. And SemVers buildmetadata using 3.9.0+next is not support by npm.

So I guess npm dist-tags are not as suitable as I initially thought.

So it it really a matter of having the list of script we need to run to build the toolkit and the package locally before we publish them.

Okay sure, I will provide infos how to bundle everything together for the npm package. Starting with ./buildToolkit ending with npm publish, correct?

[lpugin]

Any progress on this? It would be great if we can have this for 3.11, which will be out in the next couple of weeks. But no worries if not, we just need to adjust the announcement in #2897.

[WolfgangDrescher]

I could not yet find time for the pull request but it should not take too long for the main setup since I already got it working in my demo. More important will be the build process, testing and some naming decisions. But I can make it ready soon to deploy this change with 3.11. I can continue working on it early next week. Is there an estimate date for the release?

[lpugin]

Great, thanks. The tentative date for the release is July 1st.

[WolfgangDrescher]

I just opened a pull request for this: #2937