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

Update to google-closure-compiler@20240317.0.0 #96

Merged
merged 1 commit into from
Sep 30, 2024

Conversation

bpasero
Copy link
Contributor

@bpasero bpasero commented Jul 9, 2024

fix #97

👋 We (VS Code) are in the process of moving to ESM (microsoft/vscode#166033) and are running into an issue when loading jschardet.min.js into our node.js environment. The error was no longer reproducible when moving to a newer version of google-closure-compiler, in fact even just the next version v20151216 fixes it.

The error we see is when attempting to load jschardet.min.js with a custom loader we implement to bridge between ESM and CommonJS/AMD. The error is:

TypeError: Cannot read properties of undefined (reading 'iterator')
      at $jscomp.initSymbolIterator (evalmachine.<anonymous>:1:384)
      at $jscomp.makeIterator (evalmachine.<anonymous>:2:42)
      at evalmachine.<anonymous>:164:334
      at new c (evalmachine.<anonymous>:164:465)
      at evalmachine.<anonymous>:662:132
      at 42../constants (evalmachine.<anonymous>:662:395)

There is more context in microsoft/vscode#160416 (comment) and onward.

I think with newer versions of google-closure-compiler some polyfills are no longer applied that maybe lead to this issue 🤔

Curious why this project has never updated this dependency to a later version. With this PR the latest version is picked and obviously the change in jschardet.min.js is rather large. Is there a way to validate this works in all environments?

@bpasero
Copy link
Contributor Author

bpasero commented Jul 9, 2024

I was able to distill a minimal repro in #97

@bpasero
Copy link
Contributor Author

bpasero commented Sep 30, 2024

@aadsm friendly ping 👋

@aadsm aadsm merged commit f436575 into aadsm:main Sep 30, 2024
@aadsm
Copy link
Owner

aadsm commented Sep 30, 2024

Oh wow, I really missed this issue, thanks for pinging.
I was actually working on this sometime ago but then other things came up.

github-actions bot pushed a commit that referenced this pull request Sep 30, 2024
Changes since 3.1.3:
f436575 Merge pull request #96 from bpasero/update-google-closure-compiler
8437d7b 🆙 `google-closure-compiler@20240317.0.0`
7f13b6c Added missing var declaration. It was not failing tests because nodejs has no problem with it.

Bundle size changes since v3.1.3:
* dist/jschardet.js �[0;31m+4 �[0m(470244 -> 470248)
* dist/jschardet.min.js �[0;31m+2312 �[0m(339404 -> 341716)
@aadsm
Copy link
Owner

aadsm commented Sep 30, 2024

It is now available as jschardet@3.1.4.

@bpasero
Copy link
Contributor Author

bpasero commented Oct 1, 2024

Thanks a bunch!

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 this pull request may close these issues.

jschardet.min.js cannot be loaded into vm in node.js
2 participants