Skip to content

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

Open
sbc100 wants to merge 2 commits into
emscripten-core:mainfrom
sbc100:fix-closure-all-es6
Open

sbc100 wants to merge 2 commits 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 initialize GLEmulation.fogColor as [].
  • Provide default values for optional arguments across libbrowser.js, libglfw.js, libhtml5.js, libopenal.js, libsdl.js, libwasmfs.js, libfs_shared.js, libworkerfs.js, libproxyfs.js, and liblz4.js.
  • Remove surplus argument passed to GL.postDrawHandleClientVertexAttribBindings in libwebgl.js.
  • Remove unused parameter from GLImmediate.createRenderer.
  • Define source: null on SDL.music in libsdl.js.
  • Avoid this.socketEvent receiver collapse warning in libwebsocket.js.
  • In libcore.js, fix missing typeof check for globalThis.performance?.now.
  • Only generate FS_<symbol> function aliases in libfs.js for lowercase function properties.
  • 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 Outdated
@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 Outdated
Comment thread test/codesize/test_codesize_hello_dylink_all.json Outdated
@sbc100
sbc100 force-pushed the fix-closure-all-es6 branch 2 times, most recently from 43196b0 to f1713f7 Compare October 29, 2026 21:01
Unlike the other material properties (`materialAmbient`,
`materialDiffuse`, `materialSpecular`, and `materialEmission`), which
are 4-component vectors passed to `GLctx.uniform4fv`,
`materialShininess` is a single float passed to `GLctx.uniform1f`.

Store `GLEmulation.materialShininess` directly as a number initialized
to `0` (matching other scalar uniforms such as `fogStart` and
`alphaTestRef`) instead of wrapping it in a 1-element `Float32Array`.
This also avoids a Closure Compiler type warning on
`GLEmulation.materialShininess[0]` when compiling with `-sEXPORT_ES6`.

Split out from emscripten-core#27747
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 initialize `GLEmulation.fogColor` as `[]`.
- Provide default values for optional arguments across `libbrowser.js`,
  `libglfw.js`, `libhtml5.js`, `libopenal.js`, `libsdl.js`,
  `libwasmfs.js`, `libfs_shared.js`, `libworkerfs.js`, `libproxyfs.js`,
  and `liblz4.js`.
- Remove surplus argument passed to
  `GL.postDrawHandleClientVertexAttribBindings` in `libwebgl.js`.
- Remove unused parameter from `GLImmediate.createRenderer`.
- Define `source: null` on `SDL.music` in `libsdl.js`.
- Avoid `this.socketEvent` receiver collapse warning in
  `libwebsocket.js`.
- In `libcore.js`, fix missing `typeof` check for
  `globalThis.performance?.now`.
- Only generate `FS_<symbol>` function aliases in `libfs.js` for
  lowercase function properties.
- 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 f1713f7 to 21f6cb7 Compare October 29, 2026 23:08

This branch has not been deployed

No deployments
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