Skip to content

Disabling SINGLE_FILE_BINARY_ENCODE flag to prevent Binary-encoded inline wasm - #90

Open
Leafmire wants to merge 2 commits into
drumath2237:mainfrom
Leafmire:test-binary-flag-off
Open

Leafmire wants to merge 2 commits into
drumath2237:mainfrom
Leafmire:test-binary-flag-off

Conversation

@Leafmire

@Leafmire Leafmire commented Apr 10, 2026

Copy link
Copy Markdown

Summary

This PR was motivated by a report from issue:

Octal escape sequences from v0.3.1 affecting Cesium JS #89

@spz-loader/core 0.3.1 appears to trigger bundler/parser issues, while overriding back to 0.3.0 resolves the problem. One important version boundary is that packages/core/package.json in 0.3.1 pins the wasm build to emscripten/emsdk:5.0.3.

Related Emscripten issue:

emscripten-core/emscripten#25906

Based investigation, the likely cause is the single-file binary encoding format used by newer Emscripten builds default option. This PR explicitly disables that encoding and falls back to the older base64-compatible single-file output.

If the inclusion of binary encoding was not intended, this PR will revert it to the same encoding format as the previous version.

Change

add -s SINGLE_FILE_BINARY_ENCODE=0 to packages/core/lib/spz-wasm/build.sh

@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 updates the Emscripten build configuration for the spz-wasm package by disabling binary encoding for the inlined WASM file. This change is intended to resolve compatibility issues with certain bundlers, though it results in a larger bundle size due to base64 encoding. The reviewer suggests adding a comment to the build script to document this trade-off and prevent future maintainers from reverting the change without understanding its necessity.

Comment thread packages/core/lib/spz-wasm/build.sh Outdated
@WillGrayMSU

Copy link
Copy Markdown

Has anyone reviewed this PR? It would help solve an issue that is affecting CesiumJS.

@drumath2237

Copy link
Copy Markdown
Owner

@Leafmire @WillGrayMSU
Thank you for opening this PR. I'm sorry I'm late in responding.
It will take more time to review because I'm busy with work and other personal projects at the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 馃啎 New

Development

Successfully merging this pull request may close these issues.

3 participants