Skip to content

demo cleanup and modernization#2720

Closed
kevmoo wants to merge 21 commits intoWebAssembly:mainfrom
kevmoo:demo_cleanup
Closed

demo cleanup and modernization#2720
kevmoo wants to merge 21 commits intoWebAssembly:mainfrom
kevmoo:demo_cleanup

Conversation

@kevmoo
Copy link
Contributor

@kevmoo kevmoo commented Mar 15, 2026

  • emscripten: Export compact_imports feature
  • docs/demo: Modernize JS syntax and remove empty HTML tags
  • Split libwabt into separate JS and WASM files
  • Features: get features from the binary.
  • Migrate WABT web demos to use CDN dependencies
  • Add WebAssembly favicon to WABT web demos

kevmoo added 6 commits March 15, 2026 16:32
This was triggering a TypeError when instantiating the Features object in the browser demos.

Fixes an oversight from PR WebAssembly#2685 (Add initial support for compact import section. NFC) (bad0cc8) which did not update all necessary exports.
- Replaced `var` with `const` and `let` across demo scripts (wast-mode.js,
  wat2wasm/demo.js, wasm2wat/demo.js, examples.js).
- Migrated standard functions to arrow functions where context binding was
  not required.
- Refactored `debounce` and `compile` iterations for better clarity and block scoping.
- Removed an empty unused `<p>` tag from `wasm2wat/index.html`.
This is the first true WebAssembly rebuild in 4 years! By updating the CMake and Makefile configuration to output a separate `libwabt.wasm` file alongside `libwabt.js` (instead of generating a massive inlined JavaScript file), we achieve a massive file size drop in the initial JS payload.

Code Size Improvements:
* libwabt.js: Ex-size ~1.24 MB -> New size ~26 KB (a massive ~98% size drop!)
* libwabt.wasm: ~614 KB

Also updated Emscripten instantiation logic in demo scripts to correctly load the WASM binary asynchronously.
This makes the wat2wasm and wasm2wat demos extract
their default WABT feature checkboxes straight
from wabt.FEATURES instead of being hard-coded into the HTML.

Moved shared logic to share.js
Wrapped generated feature checkboxes in divs and applied flexbox layout to make them wrap cleanly in both demos.
Sorted the dynamically extracted features so they appear in alphabetical order in the demos.
Replaced locally vendored third-party CodeMirror and Split.js source files in `docs/demo/` with updated cdnjs versions (Split.js 1.6.5 and CodeMirror 5.65.18).

Extracted the pane splitter gutter graphics `horizontal.png` and `vertical.png` directly to `docs/demo/` and deleted the `docs/demo/third_party/` directory.
Downloaded the official WebAssembly logo (favicon.ico) into docs/demo and referenced it from the three demo HTML pages (index, wasm2wat, wat2wasm) so the tab displays correctly regardless of hosting path.
@kevmoo kevmoo changed the title demo cleanup demo cleanup and modernization Mar 15, 2026
@kevmoo
Copy link
Contributor Author

kevmoo commented Mar 15, 2026

CC @sbc100 – unlike the C++ changes, I actually understand most of this. 😄

@kevmoo
Copy link
Contributor Author

kevmoo commented Mar 15, 2026

I'm happy to submit these as stacked PRs, but I figured it might be easier to just land them all together if that's okay w/ you

Copy link
Member

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

Awesome! Thank for working on this.

I wonder if we should do a quick "null" deployment from main before we make these changes so we can compare before and after?

I don't think we've done a deployment of the demo in a very long time, so maybe we could do a short lived deployment before making any major changes?

I'd certainly like to see the convert to es6 emcc output and top-level await change land standalone if possible before the conversion of using node_modules

kevmoo added 12 commits March 16, 2026 17:35
…lags

- Converted wabt.post.js to use modern ES6 features (const/let, classes, etc.)
- Fixed scoping issues with _addr variables in try/finally blocks
- Added missing custom_page_sizes and wide_arithmetic exports to emscripten-exports.txt
- Removed redundant Emscripten linker flags from CMakeLists.txt
- Rebuilt demo assets (libwabt.js and libwabt.wasm)
and wrapped
Copy link
Member

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

Everything looks good in general.

As usual I'd always prefer to land stuff incrementally rather than all at one. In particular the conversions .in files seems like a large chunck that can come later.


MERGE_BASE=$(git merge-base $BRANCH HEAD)
if ! git clang-format $MERGE_BASE -q --diff -- src/ 2>&1 >/dev/null; then
if ! git clang-format $MERGE_BASE -q --diff -- src/ docs/demo/ ':(exclude)docs/demo/third_party.bundle.js' 2>&1 >/dev/null; then
Copy link
Member

Choose a reason for hiding this comment

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

I think a better way to do this is to create a .clang-format-ignore file. See https://github.com/emscripten-core/emscripten/blob/main/.clang-format-ignore

Maybe revert this file from this PR and we can add .clang-format-ignore separately.


USE_NINJA ?= 1
EMSCRIPTEN_DIR ?= $(dir $(shell which emcc))
EMSCRIPTEN_DIR ?= $(shell em-config EMSCRIPTEN_ROOT 2>/dev/null || dirname $(shell which emcc))
Copy link
Member

Choose a reason for hiding this comment

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

Is this change needed? it seems like if em-config is the PATH then emcc will always be in the PATH too.

Maybe revert this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah! this was so that folks who symlink binaries work (like on homebrew).

The directory with emcc is not necessarily the EMSCRIPTEN_ROOT

Copy link
Member

Choose a reason for hiding this comment

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

I've not heard of this... but I can see that it could be an issue. I wonder why no homebrew folks have complained about this issue in the part.

I any case, this seems like a separate bugfix that we don't need to make part of the demo upgrade.

(Seems like if do need to deal with symlinks we could also do $(realpath $(dir ($shell which emcc))) instead maybe?).

Copy link
Contributor Author

@kevmoo kevmoo Mar 17, 2026

Choose a reason for hiding this comment

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

Ran command: echo "1. \$(dirname \$(which emcc))" && echo "2. \$(realpath \$(dirname \$(which emcc)))" && echo "3. \$(dirname \$(realpath \$(which emcc)))"
Ran command: emcc_path=$(which emcc); echo "dirname:" $(dirname $emcc_path); echo "realpath dirname:" $(realpath $(dirname $emcc_path)); echo "dirname realpath:" $(dirname $(realpath $emcc_path))
Ran command: ls -la /opt/homebrew/Cellar/emscripten/5.0.3/bin/cmake

If we use $(realpath $(dir $(shell which emcc))), it actually wouldn't quite work for two reasons:

  1. Resolution order: Because dir runs before realpath, it checks dir of /opt/homebrew/bin/emcc, which returns /opt/homebrew/bin/. Calling realpath on that directory just returns /opt/homebrew/bin/ because the directory itself isn't a symlink (only the file inside it is).
  2. Homebrew's directory structure: Even if we reversed it to $(dir $(realpath $(shell which emcc))), it would resolve the symlink and return /opt/homebrew/Cellar/emscripten/5.0.3/bin. However, Homebrew places Emscripten's CMake toolchain files in libexec/cmake, not bin/cmake.

em-config EMSCRIPTEN_ROOT is a utility provided directly by Emscripten precisely for this reason! It knows its own internal paths and confidently returns the correct toolchain root (.../libexec) regardless of how the package manager scrambled the symlinks.

${WABT_SOURCE_DIR}/scripts/gen-emscripten-exports.py
${WABT_SOURCE_DIR}/include/wabt/feature.def
${WABT_SOURCE_DIR}/src/emscripten-exports.txt.in
)
Copy link
Member

Choose a reason for hiding this comment

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

In that past we've had trouble with adding python dependencies to the default build. See #1970 for example and #1385.

It might be worth leaving this exact python script stuff out of this PR and landing a version that just has the static feature list.

We can then consider this separately once this base PR has landed.

@sbc100
Copy link
Member

sbc100 commented Mar 17, 2026

In general I will always prefer to land each commit as its own PR (we always squash on landing anyway to be sure that bisect keeps working).

I'm not saying all of these commits need to be their own PR, but maybe there are at least 2 or 3 here.

@kevmoo kevmoo closed this Mar 17, 2026
@kevmoo kevmoo deleted the demo_cleanup branch March 17, 2026 19:13
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