Remove `wasm-pack` by probakowski · Pull Request #57304 · gravitational/teleport

probakowski · 2025-07-30T10:29:26Z

This change switches our build process to use wasm-bindgen directly so wasm-pack is no longer needed

zmb3

We also have a hard-coded wasm-bindgen-cli version in build.assets/windows/build.ps1 - can we update that too?

web/packages/shared/package.json

Makefile

probakowski · 2025-07-30T14:31:43Z

Tag build https://github.com/gravitational/teleport.e/actions/runs/16625484584

zmb3 · 2025-07-31T10:53:00Z

Note: this adds about 74KB to the WASM module because we're no longer stripping function names (this is something wasm-opt was doing).

We can reclaim this space by re-adding wasm-opt, by having cargo strip symbols, or by using llvm-strip or wasm-strip.

…webpack

probakowski · 2025-08-06T11:35:04Z

New tag build: https://github.com/gravitational/teleport.e/actions/runs/16758056659

tool/cargo-bin/src/main.rs

espadolini · 2025-08-06T13:56:44Z

build.assets/Dockerfile

-ARG WASM_PACK_VERSION
-# Install wasm-pack for targeting WebAssembly from Rust.
-RUN cargo install wasm-pack --locked --version ${WASM_PACK_VERSION}
-


Should we add some cache warming step here? How bad is it to compile wasm-bindgen-cli every time?

On my machine it takes 25s for wasm-bindgen-cli to be ready. Priming it is a bit tricky because cargo-bin-run depends on data from Cargo.toml and during docker image creation we don't check out code.

espadolini · 2025-08-06T13:59:11Z

web/packages/shared/libs/ironrdp/Cargo.toml

Echoing what @zmb3 said, we should enable symbol stripping for release builds here (if they're not needed for wasm-bindgen).

Symbol stripping (as in strip=true in Cargo.toml) gives no improvement in size.
I've added wasm-strip step and it gets some size back. On my machine it shows up as:

wasm-opt (size from current master): ../../../webassets/teleport/app/ironrdp_bg.wasm 436.05 kB │ gzip: 152.91 kB No wasm-stip (with or without strip in Cargo.toml): ../../../webassets/teleport/app/ironrdp_bg.wasm 558.33 kB │ gzip: 173.94 kB wasm-strip: ../../../webassets/teleport/app/ironrdp_bg.wasm 483.49 kB │ gzip: 152.40 kB

fheinecke

LGTM provided that you test by cutting a dev release after your final set of changes

fheinecke · 2025-08-07T16:41:39Z

build.assets/windows/build.ps1

    #>

-    $TempDirectoryPath = Join-Path -Path "$([System.IO.Path]::GetTempPath())" -ChildPath "$([guid]::newguid().Guid)"
+    $TempDirectoryPath = Join-Path -Path "$([System.IO.Path]::GetTempPath() )" -ChildPath "$( [guid]::newguid().Guid )"


Is this intentional (here and elsewhere)?

fheinecke · 2025-08-07T16:42:47Z

build.assets/windows/build.ps1

        $Env:CARGO_HOME = "$ToolchainDir/cargo"
        & "$ToolchainDir\rustup-init.exe" --profile minimal -y --default-toolchain "$RustVersion-x86_64-pc-windows-gnu"
        Enable-Rust -ToolchainDir $ToolchainDir
+        & rustup target add wasm32-unknown-unknown


Suggested change

& rustup target add wasm32-unknown-unknown

rustup target add wasm32-unknown-unknown

The call operator (&) is not needed here

camscale · 2025-08-08T03:22:43Z

.gitignore


 # Dockerized builds generate .pnpm-store in the root, so ignore it
 .pnpm-store
+.bin/


Why is this added to .gitignore? Can you add a comment as to why this is here and separate it from .pnmp-store as that description does not seem to apply?

camscale · 2025-08-08T03:47:51Z

web/packages/shared/package.json

  },
  "scripts": {
-    "build-wasm": "node ../../scripts/clean-up-ironrdp-artifacts.mjs && RUST_MIN_STACK=16777216 wasm-pack build ./libs/ironrdp --target web"
+    "build-wasm": "node ../../scripts/clean-up-ironrdp-artifacts.mjs && cd ./libs/ironrdp && cargo build --target wasm32-unknown-unknown --release && cp ../../../../../target/wasm32-unknown-unknown/release/ironrdp.* pkg/ && cargo bin wasm-bindgen pkg/ironrdp.wasm --out-dir pkg --typescript --target web && cargo bin wasm-strip pkg/ironrdp_bg.wasm"


This is pretty unwieldy - it is hard to see what is being run and if one part of it fails, it will be hard to tell which of the commands have failed. Is there a way to simplify this with node? If not, can we add a make target and call that?

As someone who has to chase down build failures, long run-on commands like this make life harder.

.PHONY: build-ironrdp-wasm build-ironrdp-wasm: ironrdp = web/packages/shared/libs/ironrdp build-ironrdp-wasm: node web/scripts/clean-up-ironrdp-artifacts.mjs cd $(ironrdp) && cargo build --target wasm32-unknown-unknown --release cp target/wasm32-unknown-unknown/release/ironrdp.* $(ironrdp)/pkg cargo bin wasm-bindgen $(ironrdp)/pkg/ironrdp.wasm --out-dir $(ironrdp)/pkg --typescript --target web cargo bin wasm-strip $(ironrdp)/pkg/ironrdp_bg.wasm

That will at least print out each commands as it runs so we can see what any failures would be related to. But I don't know if it makes sense to be mixing node package stuff with make stuff like this.

camscale · 2025-08-08T03:52:35Z

tool/cargo-bin/src/main.rs

I dont think this tool belongs in the top-level tool/ directory - that is for the programs we build and release. As a build tool, this belongs in build.assets/tooling or similar.

Can we also have some docs about what this is? It is simply to support the cargo bin ... command as used later? And does it just run a binary that has been put in .bin/?

If so, why do we need a tool for this - can't we just run the tool directly out of .bin/?

rosstimothy · 2025-09-17T16:29:48Z

@probakowski was this superseded by #58618?

zmb3 · 2025-09-17T16:30:35Z

Yes.

probakowski added 2 commits July 30, 2025 11:12

Use cargo-run-bin to use wasm-bindgen

f897f6a

use --target web

3815d1b

probakowski added the no-changelog Indicates that a PR does not require a changelog entry label Jul 30, 2025

probakowski changed the title ~~Remove webpack~~ Remove wasm-pack Jul 30, 2025

probakowski changed the title ~~Remove wasm-pack~~ Remove wasm-pack Jul 30, 2025

probakowski added 2 commits July 30, 2025 11:42

License

381a220

Lint

7016749

zmb3 reviewed Jul 30, 2025

View reviewed changes

web/packages/shared/package.json Outdated Show resolved Hide resolved

zmb3 reviewed Jul 30, 2025

View reviewed changes

Makefile Outdated Show resolved Hide resolved

probakowski and others added 2 commits July 30, 2025 13:36

Even more cleanup

86bd4f2

A little more cleanup

2004d5f

rustup target add wasm32-unknown-unknown

995cb03

probakowski added 2 commits August 4, 2025 22:51

Move rustup install target to builders

7a99307

Merge remote-tracking branch 'origin/master' into probakowski/remove-…

6c291a9

…webpack

probakowski mentioned this pull request Aug 4, 2025

Install wasm32 target in MacOS workflow #57525

Merged

probakowski added 2 commits August 5, 2025 10:30

Merge remote-tracking branch 'origin/master' into probakowski/remove-…

8c932d6

…webpack

Add wasm to rustup-set-version

6841591

probakowski force-pushed the probakowski/remove-webpack branch from a04036c to 6841591 Compare August 5, 2025 19:35

probakowski marked this pull request as ready for review August 6, 2025 11:33

probakowski requested review from camscale, doggydogworld, espadolini, fheinecke, fspmarshall, klizhentas, r0mant, rosstimothy and russjones as code owners August 6, 2025 11:33

probakowski requested a review from zmb3 August 6, 2025 11:33

github-actions bot added size/sm ui labels Aug 6, 2025

github-actions bot requested review from kimlisa and rudream August 6, 2025 11:34

espadolini reviewed Aug 6, 2025

View reviewed changes

probakowski added 2 commits August 7, 2025 15:47

wasm-strip

c798c28

cargo-bin attribution

33c4f4b

probakowski force-pushed the probakowski/remove-webpack branch from 0b1cef0 to 33c4f4b Compare August 7, 2025 15:37

cargo-bin attribution

d172eb6

fheinecke reviewed Aug 7, 2025

View reviewed changes

fheinecke approved these changes Aug 7, 2025

View reviewed changes

camscale reviewed Aug 8, 2025

View reviewed changes

zmb3 closed this Sep 17, 2025

zmb3 deleted the probakowski/remove-webpack branch October 29, 2025 22:36

	& rustup target add wasm32-unknown-unknown
	rustup target add wasm32-unknown-unknown

Conversation

probakowski commented Jul 30, 2025

Uh oh!

zmb3 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

probakowski commented Jul 30, 2025

Uh oh!

zmb3 commented Jul 31, 2025

Uh oh!

probakowski commented Aug 6, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fheinecke left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rosstimothy commented Sep 17, 2025

Uh oh!

zmb3 commented Sep 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

zmb3 left a comment •

edited

Loading