[js/web] fix package export for bundlers #23257

fs-eire · 2025-01-06T07:22:52Z

Description

This PR tries to fix #22615. (see detailed description in the issue)

A perfect solution would be too difficult to make, because there are a huge number of combinations of usage scenarios, including combinations of development framework, bundler, dev/prod mode, and so on.

This PR is using the following approach:

Introduce a new type of end to end test: export test. This type of tests are complete web apps that use popular web development frameworks, and the tests are using puppeteer to run the apps and check if the apps can run without error.
- added one nextjs based web app and one vite based web app.
In the test, perform the following test steps:
- npm install for packages built locally
- npm run dev to start dev server and use puppeteer to launch the browser to test
- npm run build && npm run start to test prod build and use puppeteer to launch the browser to test
Make changes to ort-web, including:
- special handling on Webpack's behavior of rewriting import.meta.url to a file:// string
- revise build definitions
- fix wasm URL for proxy, if used in a bundled build

Copilot

Copilot reviewed 30 out of 45 changed files in this pull request and generated 5 comments.

Files not reviewed (15)

js/web/test/e2e/exports/testcases/nextjs-default/.gitignore: Language not supported
js/web/test/e2e/exports/testcases/nextjs-default/app/globals.css: Language not supported
js/web/test/e2e/exports/testcases/nextjs-default/app/page.module.css: Language not supported
js/web/test/e2e/exports/testcases/nextjs-default/jsconfig.json: Language not supported
js/web/test/e2e/exports/testcases/nextjs-default/package.json: Language not supported
js/web/test/e2e/exports/testcases/nextjs-default/components/onnx-helper.js: Evaluated as low risk
js/web/test/e2e/exports/testcases/nextjs-default/components/onnx-test-bar.js: Evaluated as low risk
js/web/test/e2e/exports/testcases/nextjs-default/app/page.js: Evaluated as low risk
js/web/lib/backend-wasm.ts: Evaluated as low risk
js/web/lib/wasm/wasm-factory.ts: Evaluated as low risk
js/web/test/e2e/exports/README.md: Evaluated as low risk
js/web/lib/wasm/proxy-wrapper.ts: Evaluated as low risk
js/web/script/build.ts: Evaluated as low risk
js/web/test/e2e/exports/testcases/nextjs-default/app/layout.js: Evaluated as low risk
js/web/test/e2e/exports/testcases/nextjs-default/next.config.mjs: Evaluated as low risk

Comments suppressed due to low confidence (5)

js/web/test/e2e/exports/testcases/nextjs-default.md:15

[nitpick] The term 'App Router' should be 'app router' to match the casing used in the rest of the document.

√ Would you like to use App Router? (recommended) ... No

js/web/test/e2e/exports/main.js:30

Ensure that the ANSI escape codes for colored output are intentional and correctly handled in environments that might not support them.

await runDevTest('vite-default', '\x1b[32m➜\x1b[39m  \x1b[1mLocal\x1b[22m:', 5173);

js/web/test/e2e/exports/main.js:31

Ensure that the ANSI escape codes for colored output are intentional and correctly handled in environments that might not support them.

await runProdTest('vite-default', '\x1b[32m➜\x1b[39m  \x1b[1mLocal\x1b[22m:', 4173);

js/web/lib/wasm/wasm-utils-import.ts:1

The preload function should have an explicit return type defined for better readability and maintainability.

preload = async (absoluteUrl: string): Promise<string> => {

js/web/lib/wasm/wasm-utils-import.ts:12

[nitpick] The error handling in the normalizeUrl function should provide more context to help with debugging.

normalizeUrl = (filename: string, prefixOverride?: string) => {

js/web/test/e2e/exports/testcases/nextjs-default.md

js/web/test/e2e/exports/test.js

### Description  This PR tries to fix #22615. (see detailed description in the issue) A perfect solution would be too difficult to make, because there are a huge number of combinations of usage scenarios, including combinations of development framework, bundler, dev/prod mode, and so on. This PR is using the following approach: - Introduce a new type of end to end test: export test. This type of tests are complete web apps that use popular web development frameworks, and the tests are using puppeteer to run the apps and check if the apps can run without error. - added one nextjs based web app and one vite based web app. - In the test, perform the following test steps: - `npm install` for packages built locally - `npm run dev` to start dev server and use puppeteer to launch the browser to test - `npm run build && npm run start` to test prod build and use puppeteer to launch the browser to test - Make changes to ort-web, including: - special handling on Webpack's behavior of rewriting `import.meta.url` to a `file://` string - revise build definitions - fix wasm URL for proxy, if used in a bundled build

xenova · 2025-01-15T15:24:56Z

I upgraded but ran into an issue where the ort-wasm-simd-threaded.jsep.mjs file was not included in the bundle.

huggingface/transformers.js#1150

fs-eire · 2025-01-15T19:20:02Z

I upgraded but ran into an issue where the ort-wasm-simd-threaded.jsep.mjs file was not included in the bundle.

huggingface/transformers.js#1150

Did you override env.wasm.wasmPaths in transformer.js?

The expected build-time behavior is, if importing onnxruntime-web as ESM, the file ort.bundle.min.js (which includes ort-wasm-simd-threaded.jsep.mjs) is picked for processing. all the content will be a part of dist/transformers.js

The expected run-time behavior is:

If specified env.wasm.wasmPaths:
1. If specified as a string (URL prefix): it will always try to dynamically load {PREFIX}/ort-wasm-simd-threaded.jsep.mjs and {PREFIX}/ort-wasm-simd-threaded.jsep.wasm
2. If specified as an object { mjs: "{MJS_URL}" }: will dynamically load {MJS_URL} and infer URL of wasm from {MJS_URL}
3. If specified as an object { wasm: "{WASM_URL}" }: will use bundled mjs and will load wasm from {WASM_URL}
4. If specified as an object { mjs: "{MJS_URL}", wasm: "{WASM_URL}" }: will load mjs and wasm using the specified URL
If not specified env.wasm.wasmPaths: will use the bundled mjs and use import.meta.url to infer URL of wasm

I think transformer.js explicitly set env.wasm.wasmPaths to a CDN prefix so it's the case (1). It should be able to make it not depend on this file if using (3).

fs-eire · 2025-01-15T19:21:30Z

@xenova BTW, transformerjs 3.3.1 failed to build on windows because rm and cp is not available

xenova · 2025-01-15T19:24:20Z

@xenova BTW, transformerjs 3.3.1 failed to build on windows because rm and cp is not available

Thanks for spotting! I had to publish a hot-fix since there were issues when running in-browser, so I'll revert this soon and integrate a more permanent fix (hopefully after onnxruntime-web is fixed)?

fs-eire · 2025-01-15T19:52:20Z

@xenova BTW, transformerjs 3.3.1 failed to build on windows because rm and cp is not available

Thanks for spotting! I had to publish a hot-fix since there were issues when running in-browser, so I'll revert this soon and integrate a more permanent fix (hopefully after onnxruntime-web is fixed)?

I assume latest dev package should have no problem. Please let me know if you observed any unexpected behavior (both build time and run time)

xenova · 2025-01-15T19:58:29Z

The issue (which also happens with https://www.npmjs.com/package/onnxruntime-web/v/1.21.0-dev.20250114-228dd16893) appears to be that webpack is unable to identify that the .mjs file needs to be linked too, so when building transformers.js, that file isn't copied correctly.

Similarly, the bundle file is copied, even though it's not necessary since transformers.js includes it.

fs-eire · 2025-01-15T21:30:04Z

The issue (which also happens with https://www.npmjs.com/package/onnxruntime-web/v/1.21.0-dev.20250114-228dd16893) appears to be that webpack is unable to identify that the .mjs file needs to be linked too, so when building transformers.js, that file isn't copied correctly.

Similarly, the bundle file is copied, even though it's not necessary since transformers.js includes it.

I think this is expected. As I explained above, when webpack builds transformer.js, it finds import * from "onnxruntime-web" and this resolves to ort.bundle.min.mjs. "ort.bundle.min.mjs" is a file that already contains the content of ort-wasm-simd-threaded.jsep.mjs, so it does not depends on file ort-wasm-simd-threaded.jsep.mjs. It is expected that webpack does not copy this file.

This should work for bundlers if env.wasm.wasmPaths is not set, or it's set to an object which contains only "wasm" property (as described above for case (3))

if env.wasm.wasmPaths is set to a path prefix or an object containing "mjs" property (case 1, 2, 4), it means user want to explicitly set the path of the ort-wasm-simd-threaded.jsep.mjs file. ort.bundle.min.mjs will ignore its bundled mjs and load from the specific path instead.

xenova · 2025-01-15T22:02:38Z

The issue (which also happens with https://www.npmjs.com/package/onnxruntime-web/v/1.21.0-dev.20250114-228dd16893) appears to be that webpack is unable to identify that the .mjs file needs to be linked too, so when building transformers.js, that file isn't copied correctly.
Similarly, the bundle file is copied, even though it's not necessary since transformers.js includes it.

I think this is expected. As I explained above, when webpack builds transformer.js, it finds import * from "onnxruntime-web" and this resolves to ort.bundle.min.mjs. "ort.bundle.min.mjs" is a file that already contains the content of ort-wasm-simd-threaded.jsep.mjs, so it does not depends on file ort-wasm-simd-threaded.jsep.mjs. It is expected that webpack does not copy this file.

This should work for bundlers if env.wasm.wasmPaths is not set, or it's set to an object which contains only "wasm" property (as described above for case (3))

if env.wasm.wasmPaths is set to a path prefix or an object containing "mjs" property (case 1, 2, 4), it means user want to explicitly set the path of the ort-wasm-simd-threaded.jsep.mjs file. ort.bundle.min.mjs will ignore its bundled mjs and load from the specific path instead.

Ah I see - thanks for that explanation. So, do you have any recommendations for how to improve this in Transformers.js? If possible, I'd like to bundle everything non-wasm related into the transformers.js file. If you know how to do this, would you consider opening a PR? Otherwise, I can give it a go.

fs-eire · 2025-01-15T22:38:13Z

The issue (which also happens with https://www.npmjs.com/package/onnxruntime-web/v/1.21.0-dev.20250114-228dd16893) appears to be that webpack is unable to identify that the .mjs file needs to be linked too, so when building transformers.js, that file isn't copied correctly.
Similarly, the bundle file is copied, even though it's not necessary since transformers.js includes it.

I think this is expected. As I explained above, when webpack builds transformer.js, it finds import * from "onnxruntime-web" and this resolves to ort.bundle.min.mjs. "ort.bundle.min.mjs" is a file that already contains the content of ort-wasm-simd-threaded.jsep.mjs, so it does not depends on file ort-wasm-simd-threaded.jsep.mjs. It is expected that webpack does not copy this file.
This should work for bundlers if env.wasm.wasmPaths is not set, or it's set to an object which contains only "wasm" property (as described above for case (3))
if env.wasm.wasmPaths is set to a path prefix or an object containing "mjs" property (case 1, 2, 4), it means user want to explicitly set the path of the ort-wasm-simd-threaded.jsep.mjs file. ort.bundle.min.mjs will ignore its bundled mjs and load from the specific path instead.

Ah I see - thanks for that explanation. So, do you have any recommendations for how to improve this in Transformers.js? If possible, I'd like to bundle everything non-wasm related into the transformers.js file. If you know how to do this, would you consider opening a PR? Otherwise, I can give it a go.

The major possible use cases are:

import from CDN directly
This may happen in some examples that uses transformers.js directly in a <script> tag.
import in a webpack based web app framework
For example, a typical Next.js app
import in a rollup/esbuild based web app framework
For example, a typical Vite/Nuxt app
import inside a service worker

Could you find me some tranformersjs examples links for those use case, so that I can start work on them?

xenova · 2025-01-16T00:14:20Z

Sure, here's a repo containing our example projects: https://github.com/huggingface/transformers.js-examples/tree/migrate-examples (on the migrate-examples branch) since I'm in the progress of moving some repos from the transformers.js repo to there.

Some examples:

fs-eire added 11 commits December 19, 2024 13:04

working

dfa3e0c

ex

a3c16a0

ex2

75b0412

fix package install

e8247d5

Merge remote-tracking branch 'origin/main' into fs-eire/w-exp

ef8ed13

[js/web] fix package export for bundlers

9f365b4

add test case for Turbopack

48dc9b8

fix proxy

cb08d72

revise test and add vite test

eee8a5c

fix test

649f81e

format

cbd1ea8

fs-eire marked this pull request as ready for review January 6, 2025 09:47

fs-eire requested review from Copilot and guschmue January 6, 2025 09:48

Copilot AI reviewed Jan 6, 2025

View reviewed changes

fs-eire added 5 commits January 6, 2025 17:28

fix usage in cors

83be4f8

Merge remote-tracking branch 'origin/main' into fs-eire/web-export

3878a1b

simplify nextjs-default

c664288

simplify vite-default

2f85ddc

revise document

2d0db3d

guschmue approved these changes Jan 9, 2025

View reviewed changes

fs-eire merged commit 0627a6c into main Jan 9, 2025
61 of 63 checks passed

fs-eire deleted the fs-eire/web-export branch January 9, 2025 19:01

fs-eire mentioned this pull request Jan 14, 2025

[Web] Error when using Web Workers on Next.js #22113

Open

fs-eire mentioned this pull request Jan 16, 2025

[js/web] make ort-web export compatible with webpack #22196

Closed

xenova mentioned this pull request Jan 16, 2025

Next.js example breaks with v3 huggingface/transformers.js#911

Closed

This was referenced Jan 19, 2025

[js/web] ESM: use the bundled target as default export #20991

Merged

[DO NOT MERGE] fix package export huggingface/transformers.js#1161

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[js/web] fix package export for bundlers #23257

[js/web] fix package export for bundlers #23257

fs-eire commented Jan 6, 2025 •

edited

Loading

Copilot AI left a comment

xenova commented Jan 15, 2025

fs-eire commented Jan 15, 2025

fs-eire commented Jan 15, 2025

xenova commented Jan 15, 2025

fs-eire commented Jan 15, 2025

xenova commented Jan 15, 2025 •

edited

Loading

fs-eire commented Jan 15, 2025 •

edited

Loading

xenova commented Jan 15, 2025

fs-eire commented Jan 15, 2025 •

edited

Loading

xenova commented Jan 16, 2025

[js/web] fix package export for bundlers #23257

[js/web] fix package export for bundlers #23257

Conversation

fs-eire commented Jan 6, 2025 • edited Loading

Description

Copilot AI left a comment

Choose a reason for hiding this comment

xenova commented Jan 15, 2025

fs-eire commented Jan 15, 2025

fs-eire commented Jan 15, 2025

xenova commented Jan 15, 2025

fs-eire commented Jan 15, 2025

xenova commented Jan 15, 2025 • edited Loading

fs-eire commented Jan 15, 2025 • edited Loading

xenova commented Jan 15, 2025

fs-eire commented Jan 15, 2025 • edited Loading

xenova commented Jan 16, 2025

fs-eire commented Jan 6, 2025 •

edited

Loading

xenova commented Jan 15, 2025 •

edited

Loading

fs-eire commented Jan 15, 2025 •

edited

Loading

fs-eire commented Jan 15, 2025 •

edited

Loading