Skip to content

[JS] Fix Closure compiler arity and type warnings with -sEXPORT_ES6 - #28100

Merged
sbc100 merged 1 commit into
emscripten-core:mainfrom
sbc100:fix-closure-all-es6
Oct 30, 2026
Merged

sbc100 merged 1 commit into
emscripten-core:mainfrom
sbc100:fix-closure-all-es6

Conversation

@sbc100

@sbc100 sbc100 commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

Followup to #27749.

Make all remaining JS libraries compatible with Closure compiler in
ES6 module mode (-sEXPORT_ES6).

  • Change loop variables in $miniTempWebGLFloatBuffers__postset and
    $miniTempWebGLIntBuffers__postset from var to let to prevent
    duplicate top-level var declaration collisions in ES module scope.
  • Remove duplicate forward var declarations in
    $GLEmulation__postset and fogColor: null initializer on
    $GLEmulation.
  • Provide default values for optional arguments across libbrowser.js,
    libfs_shared.js, libglemu.js, libglfw.js, libhtml5.js,
    liblz4.js, libopenal.js, libproxyfs.js, libsdl.js,
    libwasmfs.js, and libworkerfs.js.
  • Remove unused parameter from GLImmediate.createRenderer.
  • Remove undefined SDL.music.source reference in libsdl.js.
  • Update test_closure_full_js_library to run with -sEXPORT_ES6 and
    remove temporary test_closure_fs_es6.

See: #27747

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces significant updates to the library files to improve ES6 compatibility and default parameter handling, including the transition to ES6 module output in tests. A critical issue was identified in src/lib/libglemu.js where initializing materialShininess to an empty array [] changes its truthiness and could lead to runtime errors; the reviewer suggests using a safe default value like [0.0] instead.

Comment thread src/lib/libglemu.js Resolved
@sbc100
sbc100 force-pushed the fix-closure-all-es6 branch from 341d848 to 5566eaa Compare October 29, 2026 17:40
Comment thread src/lib/libglemu.js Resolved
@sbc100
sbc100 force-pushed the fix-closure-all-es6 branch 4 times, most recently from 21f6cb7 to 8e5c14b Compare October 30, 2026 18:45
@sbc100
sbc100 requested a review from kripken October 30, 2026 18:45
@sbc100
sbc100 force-pushed the fix-closure-all-es6 branch from 8e5c14b to a717583 Compare October 30, 2026 18:49
Followup to emscripten-core#27749.

Make all remaining JS libraries compatible with Closure compiler in
ES6 module mode (`-sEXPORT_ES6`).

- Change loop variables in `$miniTempWebGLFloatBuffers__postset` and
  `$miniTempWebGLIntBuffers__postset` from `var` to `let` to prevent
  duplicate top-level `var` declaration collisions in ES module scope.
- Remove duplicate forward `var` declarations in
  `$GLEmulation__postset` and `fogColor: null` initializer on
  `$GLEmulation`.
- Provide default values for optional arguments across `libbrowser.js`,
  `libfs_shared.js`, `libglemu.js`, `libglfw.js`, `libhtml5.js`,
  `liblz4.js`, `libopenal.js`, `libproxyfs.js`, `libsdl.js`,
  `libwasmfs.js`, and `libworkerfs.js`.
- Remove unused parameter from `GLImmediate.createRenderer`.
- Remove undefined `SDL.music.source` reference in `libsdl.js`.
- Update `test_closure_full_js_library` to run with `-sEXPORT_ES6` and
  remove temporary `test_closure_fs_es6`.

See: emscripten-core#27747
@sbc100
sbc100 force-pushed the fix-closure-all-es6 branch from a717583 to ae0c9ae Compare October 30, 2026 20:37
@sbc100
sbc100 merged commit d78a026 into emscripten-core:main Oct 30, 2026
18 of 24 checks passed
@sbc100
sbc100 deleted the fix-closure-all-es6 branch October 30, 2026 20:48
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.

2 participants