Repository navigation
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modernizes the codebase by adopting several ES6+ features and enforcing them via new ESLint rules. Key changes include replacing Math.pow with the exponentiation operator (**), utilizing Array.from and .slice() instead of Array.prototype.slice.call, adopting .at(-1) for accessing the last element of arrays, replacing indexOf checks with .includes(), and using logical assignment operators (??=, ||=). Additionally, some plain objects were refactored to use Map. Feedback on these changes highlights potential issues with the new logical assignment operators. Specifically, using ??= on objects or constructors (such as derivedClass.constructor and SDL.joystickNamePool) can bypass assignments if the property name collides with inherited prototype properties (e.g., toString or constructor). Reverting to Object.hasOwn checks is recommended in these cases. Furthermore, converting a typed array to a standard array via Array.from(arg).slice(0, 400) in src/runtime_debug.js is highly inefficient for large arrays; slicing the typed array first with .subarray(0, 400) before calling Array.from is suggested to avoid unnecessary memory copying.
Historically, `Array.prototype.slice.call` was used in two places in our JS runtime and library code: 1. On `TypedArray` instances (in `libglemu.js`, `libmemfs.js`, and `runtime_debug.js`). Before ES2015 incorporated `TypedArray` into the ECMAScript standard, the Khronos WebGL `TypedArray` specification lacked `%TypedArray%.prototype.slice()` for shallow copying as well as `%TypedArray%.prototype.toString()` for comma-separated formatting (calling `.toString()` inherited `Object.prototype.toString` and produced `"[object Float32Array]"`). Converting to a plain `Array` via `Array.prototype.slice.call` was therefore necessary both to clone matrices in `glPushMatrix` and to format typed arrays in debug logs. 2. On `arguments` objects (in `jsifier.mjs` and `libcore.js`) before `Array.from()` was introduced in ES2015. `%TypedArray%.prototype.slice()`, `%TypedArray%.prototype.toString()`, and `Array.from()` have all been supported since Chrome 45, Firefox 38, Safari 10, and Node.js 4.0 (2015-2016), well before Emscripten's minimum supported engine versions (Chrome 85, Firefox 79, Safari 15, Node 18.3). In `runtime_debug.js`, calling `arg.slice(0, 400)` directly also avoids copying the entire typed array before slicing to 400 elements, and in `glPushMatrix` it preserves the `Float32Array` type on the matrix stack.
Follow-up to emscripten-core#28129: - Enable `prefer-exponentiation-operator`, `prefer-object-spread`, `prefer-rest-params`, `prefer-spread`, and `logical-assignment-operators` in `eslint.config.mjs`. - Use `Object.values`, direct integer indexing, or `??=` in places where `$hasOwn` is not needed. - Use `Map` instead of plain `{}` objects for lookup tables in `acorn-optimizer.mjs` and `libglfw.js`. - Replace legacy `.apply`, `.indexOf`, `Math.pow`, and `arr[arr.length - 1]` idioms with `.call` / spread syntax, `.includes`, `**` / `Math.hypot`, and `.at(-1)`.
Follow-up to #28129 and #28139
prefer-exponentiation-operator,prefer-object-spread,prefer-rest-params,prefer-spread, andlogical-assignment-operatorsineslint.config.mjs.Object.values, direct integer indexing, or??=in places where$hasOwnis not needed.Mapinstead of plain{}objects for lookup tables inacorn-optimizer.mjsandlibglfw.js.Array.prototype.slice.call,.apply,.indexOf,Math.pow, andarr[arr.length - 1]idioms with.slice,Array.from, spread syntax,.includes,**/Math.hypot, and.at(-1).