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

Issue with source map in v0.2.37 when running with Vite #414

Closed
felladrin opened this issue May 22, 2024 · 7 comments · Fixed by #420
Closed

Issue with source map in v0.2.37 when running with Vite #414

felladrin opened this issue May 22, 2024 · 7 comments · Fixed by #420

Comments

@felladrin
Copy link
Contributor

felladrin commented May 22, 2024

When running the project with Vite, it's throwing this error:
(It doesn't prevent Vite from running, it's just annoying)

  VITE v5.2.11  ready in 257 ms

  ➜  Local:   http://localhost:7860/
  ➜  Network: http://172.19.0.2:7860/
✘ [ERROR] Expected "," in source map but found "MLC_DUMMY_REQUIRE_VAR"

    node_modules/@mlc-ai/web-llm/lib/index.js.map:1:212952:
      1 │ ...st performanceNode = "MLC_DUMMY_REQUIRE_VAR";\r\n            ret...
        │                          ~~~~~~~~~~~~~~~~~~~~~
        ╵                          ,

  The source map "node_modules/@mlc-ai/web-llm/lib/index.js.map" was referenced by the file "node_modules/@mlc-ai/web-llm/lib/index.js" here:

    node_modules/@mlc-ai/web-llm/lib/index.js:8190:21:
      8190 │ //# sourceMappingURL=index.js.map
           ╵                      ~~~~~~~~~~~~
image

Temporary solution: remove the //# sourceMappingURL=index.js.map line from @mlc-ai/web-llm/lib/index.js before compiling and Vite won't throw any errors.


Extra info: The v0.2.35 didn't have this problem.

@felladrin felladrin changed the title Issue with source map in v0.2.37 when building with Vite Issue with source map in v0.2.37 when running with Vite May 22, 2024
@CharlieFRuan
Copy link
Contributor

Thanks for reporting the error, will send a fix soon

CharlieFRuan added a commit that referenced this issue May 22, 2024
Address issue #414. Since
`index.js.map` is a json, when specifying string, we should use `\"`
instead of `"`.

Prior to this PR, we populate `const performanceNode =
"MLC_DUMMY_REQUIRE_VAR"`, this PR changes it to `const performanceNode =
\"MLC_DUMMY_REQUIRE_VAR\"`
CharlieFRuan added a commit that referenced this issue May 22, 2024
### Changes
The only two changes are:

- #413
- #415
- Fixes `index.js.map` issue in Vite reported in
#414

### WASM Version
v0_2_34 as no change is required.

### TVMjs
TVMjs compiled at
apache/tvm@a5862a5,
with no change
@CharlieFRuan
Copy link
Contributor

Could you try 0.2.38? Should be fixed via #415. Apologies for the inconvenience

@felladrin
Copy link
Contributor Author

felladrin commented May 24, 2024

Thanks for the quick response, @CharlieFRuan!

I've installed v0.2.38 and noticed a difference already, but now the error is in another place of index.js.map:

image

I've manually replaced all occurrences of "MLC_DUMMY_PATH" with \"MLC_DUMMY_PATH\" in the source map and confirmed it runs without that error.
So using the same solution from #415 for this variable should work. (Proposed in #420)

@felladrin
Copy link
Contributor Author

Sorry, @CharlieFRuan and @tqchen,
It seems the fix on #420 didn't solve the problem with "MLC_DUMMY_PATH".
I just confirmed that this issue is present on v0.2.39:

image

I also double-checked it on https://www.npmjs.com/package/@mlc-ai/web-llm?activeTab=code and we can see the issue is there on v0.2.39 release:

image

Can we re-open this issue, and could you confirm why the change on #420 didn't fix it?

@CharlieFRuan
Copy link
Contributor

CharlieFRuan commented May 31, 2024

Ahh thanks for the catch; this should be due to how we need ' instead of ". Will send a patch now and fix in 0.2.40

@CharlieFRuan CharlieFRuan reopened this May 31, 2024
CharlieFRuan added a commit that referenced this issue May 31, 2024
This PR fixes `\"` issue in `index.js.map`, which breaks Vite
deployment, follows the thread
#414

This PR undoes the change in `cleanup-index-js.sh` to `index.js` in
#420, and update `"` to `'` for
`index.js.map`.
@CharlieFRuan
Copy link
Contributor

Could you try 0.2.40 which includes #440?

felladrin added a commit to felladrin/MiniSearch that referenced this issue May 31, 2024
@felladrin
Copy link
Contributor Author

Thank you! Now it's fixed! 🎉

Confirmed here:

image

And also by removing the workaround I had for the issue.

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 a pull request may close this issue.

2 participants