Skip to content

Fix browser entry in bundlers without wasm ESM support (npm 1.0.16) - #56

Merged
ianrumac merged 11 commits into
masterfrom
ir/fix/browser-wasm-bundler-fallback
Sep 9, 2026
Merged

ianrumac merged 11 commits into
masterfrom
ir/fix/browser-wasm-bundler-fallback

Conversation

@ianrumac

@ianrumac ianrumac commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Problem

The npm package's /browser entry is wasm-pack --target bundler output, which does import * as wasm from './superscript_bg.wasm'. That import shape only works in bundlers with WebAssembly ESM integration (webpack asyncWebAssembly, vite-plugin-wasm). Bundlers that instead resolve .wasm imports as plain file assets (Bun by default, esbuild with --loader:.wasm=file) get a URL string where wasm exports should be, so wasm.__wbindgen_start() throws at import time:

wasm.__wbindgen_start is not a function

In Superwall-Web this made every audience expression evaluation fail (falling back to fail-open matching).

Fix

browser.ts now tries the existing bundler-target import first, and on failure falls back to a new wasm-pack --target web build initialised from base64-inlined wasm bytes (scripts/inline-wasm.ts generates the inline module at build time). The inline path needs no bundler wasm/asset support at all. If both paths fail, the thrown error carries both underlying errors so field reports stay diagnosable.

Both paths sit behind dynamic imports, so wasm-capable bundlers code-split the ~1.7 MB inline chunk and never fetch it on the happy path. Also removes noisy console.log calls from the browser host-context callbacks.

Scope note: bundlers with no .wasm handling at all fail at build time before either path can run — unchanged from previous releases. Verified: default esbuild aborts with No loader is configured for ".wasm" files; per review, Next.js' default webpack config (webpack 5.98, no asyncWebAssembly) also fails at build time. Those consumers need --loader:.wasm=file resp. experiments.asyncWebAssembly: true, after which they are covered (esbuild via the new fallback, webpack via the primary path).

Versioning

npm publishes had diverged from repo versioning (last npm: 1.0.3, repo: 1.0.15). This bumps the npm package and the wasm wrapper crate to 1.0.16 to realign. Root Cargo.toml is untouched (native pipeline owns it).

CI fix

examples/browser's bun.lock only recorded @rollup/rollup-darwin-arm64, so the Linux runner failed with Cannot find module @rollup/rollup-linux-x64-gnu. Regenerated the lockfile (records all platform variants), removed the stale bun.lockb/package-lock.json, and replaced vite-plugin-top-level-await (breaks against current @swc/core: missing field type) with build.target: 'esnext'.

Verified

  • Bun-bundled build (previously broken): bundler path fails, fallback loads, (size(device.activeEntitlements) == 0) && (params.event_name == "test_embed_redirect") evaluates to {"Ok":{"type":"bool","value":true}}
  • esbuild with --loader:.wasm=file: builds, fallback path evaluates correctly; default esbuild fails at build time (pre-existing, documented)
  • webpack asyncWebAssembly build: compiles clean, happy path unchanged, fallback chunk never fetched
  • /node entry: unchanged, evaluates correctly
  • examples/browser: bun install && CI=false bun run build succeeds

The /browser entry (wasm-pack --target bundler output) does
'import * as wasm from ./superscript_bg.wasm', which bundlers like Bun
and esbuild resolve as a file asset instead of a wasm module, so
wasm.__wbindgen_start() throws at import time and evaluation never runs
(Superwall-Web then falls back to fail-open matching).

browser.ts now tries the bundler-target import first and falls back to
a new --target web build initialised from base64-inlined wasm bytes,
which needs no bundler wasm/asset support. The fallback is behind a
dynamic import so wasm-capable bundlers (webpack asyncWebAssembly,
vite-plugin-wasm) code-split it and never fetch it.

Also removes noisy console.log calls from the browser host-context
callbacks, and aligns the npm package version (previously lagging at
1.0.3) and the wasm wrapper crate with the repo versioning at 1.0.16.

Co-authored-by: Cursor <cursoragent@cursor.com>

@pullfrog pullfrog 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.

Important

The loading mechanism is sound and I verified the wasm-bindgen API usage against the actual generated glue — but a runtime .catch() around a dynamic import() cannot rescue a bundler that fails at build time, and two of the three toolchains named in the CHANGELOG appear to fail that way.

Reviewed changes — full diff of the single commit f029916 against master, plus the npm build/publish workflows, build_wasm.sh, wasm/tsconfig.json, and the examples/browser / examples/node consumers.

  • Two-path browser loaderwasm/src/browser.ts now tries the --target bundler glue and falls back to a --target web build initialised from base64-inlined bytes, memoized in a single wasmModulePromise.
  • Inline wasm generator — new Bun script wasm/scripts/inline-wasm.ts emits target/web/superscript_bg_inline.{js,d.ts} at build time.
  • Build pipelinebuild:wasm:web + generate:inline scripts added; copy:wasm and build extended to cover target/web.
  • Version realignmentwasm/package.json 1.0.2 → 1.0.16, wasm/Cargo.toml 1.0.12 → 1.0.16, root Cargo.toml deliberately untouched at 1.0.15.
  • Logging cleanup — the two console.log calls in the browser host-context callbacks are removed.

Things I checked and found correct, so they don't need defending: glue.default({ module_or_path: binary }) is the current wasm-bindgen object form and is stable across the whole ^0.2.93 range this unpinned manifest resolves; #[wasm_bindgen(start)] does run on the raw-bytes path; the web glue's new URL(..., import.meta.url) branch is dead when explicit bytes are supplied; build:wasm:webbuild:tscopy:wasm ordering makes the generated .d.ts available to tsc and lands the artifacts where dist/{esm,cjs}/browser.js expect them; and both npm workflows set up Bun before build_wasm.sh.

⚠️ Nothing exercises the fallback path

examples/browser/src/SuperscriptParser.jsx does import @superwall/superscript/browser, so src/browser.ts is compiled in CI — but the example uses vite-plugin-wasm, which means only loadBundlerModule() is ever taken, and the workflow's assertion is if [ ! -d dist ]. Nothing executes evaluateWithContext in a browser-like runtime, so a truncated base64 blob, a wasm-bindgen init-signature drift, or a broken inline-wasm.ts run would all publish green. That is a sharp edge for a fix whose entire value is a path CI can't see.

Technical details
# No CI coverage for the inline-wasm fallback

## Affected sites
- `.github/workflows/build-test-PR-superscript-npm.yml` — browser example step only asserts `dist/` exists after `vite build`; nothing runs.
- `.github/workflows/build-test-publish-superscript-npm.yml` — same steps, then publishes.
- `examples/browser/package.json` — depends on `vite-plugin-wasm`, so the primary path always wins.
- `examples/node/tests/superscript.test.ts` — only exercises the `/node` entry.

## Required outcome
- CI must fail if `loadInlineModule()` cannot initialise the wasm module and evaluate an expression.

## Suggested approach (optional)
- The cheapest option is a standalone check that does not need a bundler at all: after `npm run build`, run a small script under Bun/Node that imports `wasm/dist/target/web/superscript.js` + `superscript_bg_inline.js`, performs the same `atob``Uint8Array``init` sequence, and asserts a known expression evaluates to `{"Ok":{"type":"bool","value":true}}`.
- A second option that also covers the real integration: add a `bun build`-bundled fixture app (the bundler this PR actually fixes) and execute its output, rather than only building it.

ℹ️ Nitpicks

  • wasm/package.json:33copy:wasm ships dist/target/web/superscript_bg.wasm, but no code path reads it: loadInlineModule instantiates from the base64 copy. With wasm-opt = false, the binary now ships three times plus a ~1.33× base64 copy in the tarball.
  • wasm/package.json:28-29npm run build (and therefore prepublishOnly) now hard-requires Bun. CI has it, but a maintainer publishing locally gets bun: command not found. Worth a line in wasm/BUILDING.md or an engines note.
  • wasm/src/node.ts:31,39 still has the console.log("Computed property result in node", ...) pair that this PR removes from browser.ts. Same noise, same fix.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread CHANGELOG.md Outdated
Comment thread wasm/src/browser.ts Outdated
- CHANGELOG + browser.ts docs: only claim toolchains the runtime
  fallback demonstrably reaches. Bun works out of the box; esbuild needs
  --loader:.wasm=file (verified: build fails without it, fallback works
  with it); default Next.js webpack config fails at build time as
  before and needs experiments.asyncWebAssembly.
- browser.ts: preserve both load-path errors when bundler and inline
  paths fail, instead of memoizing only the inline rejection.
- examples/browser: regenerate bun.lock (old one only recorded
  @rollup/rollup-darwin-arm64, breaking Linux CI with 'Cannot find
  module @rollup/rollup-linux-x64-gnu'); drop stale bun.lockb and
  package-lock.json; replace vite-plugin-top-level-await (incompatible
  with current @swc/core: 'missing field type') with
  build.target esnext.

Co-authored-by: Cursor <cursoragent@cursor.com>

@pullfrog pullfrog 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.

ℹ️ No critical issues — one cleanup inline.

Reviewed changes — the delta since the prior pullfrog review at f029916, i.e. commit 3093af6, plus the npm workflows, wasm/tsconfig.json, and the examples/browser consumer.

  • Toolchain claims narrowed — the CHANGELOG.md 1.0.16 entry and the loadInlineModule doc comment now claim only bundlers that resolve .wasm as a file asset (Bun by default, esbuild with --loader:.wasm=file), and explicitly record that default esbuild and Next.js' default webpack config fail at build time before either path runs. That matches the mechanism: the bundler-target glue's import * as wasm from './superscript_bg.wasm' is statically resolved even behind a dynamic import(), so the .catch only rescues the runtime-failure group.
  • Both load failures preservedwasm/src/browser.ts:46-69 aggregates the bundler and inline errors into one Error, interpolated into the message and attached as bundlerError / inlineError. The prior review's { cause } suggestion would not have compiled here — wasm/tsconfig.json has target: ES2020 with no lib override, so ErrorOptions is not in scope; the property-assignment form is the right call.
  • examples/browser CI unblockedbun.lock regenerated (now records all platform variants of @rollup/* and @swc/core-*, fixing Cannot find module @rollup/rollup-linux-x64-gnu), stale bun.lockb and package-lock.json deleted, and vite-plugin-top-level-await replaced with build: { target: 'esnext' }. build-test-publish is green on 3093af6.

I checked the lockfile regeneration in detail and it is consistent: no dependency the example's source needs is missing, every version bump stays inside the ^/~ ranges already declared in package.json, and vite-plugin-top-level-await's transitive-only packages (@rollup/plugin-virtual, uuid) are removed cleanly rather than orphaned. The commander@2.20.3commander@10.0.1 move at the flat packages key is a relabel, not a resolution change — both versions are present before and after.

ℹ️ Nitpicks

  • examples/browser/vite.config.ts:12target: 'esnext' disables all syntax downleveling, not just the top-level-await wrapper, so the demo bundle now needs Chrome 89+ / Safari 15+ where the plugin previously downlevelled it. Fine for a playground app; worth knowing if it is deployed anywhere user-facing.
  • wasm/src/browser.ts:57-64 — the four-line error as Error & { … } cast is spelled out twice; Object.assign(error, { bundlerError, inlineError }) is the same thing in one line and type-checks under lib: ES2020.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread examples/browser/package.json Outdated
ianrumac and others added 2 commits September 8, 2026 15:53
If the bundler-target import and the base64-inline web target both fail,
fetch the exact-version superscript_bg.wasm from jsDelivr and initialise
the local --target web glue with it. Pin the URL to package.json's
version (generated into src/version.ts) so the binary matches the glue.
Override with globalThis.SUPERWALL_SUPERSCRIPT_WASM_URL to self-host.

Verified: web-target init from a hosted URL evaluates
(size(device.activeEntitlements) == 0) && (params.event_name == ...)
to {Ok: {type: bool, value: true}}. This is runtime-only — bundlers
that fail at build time on .wasm still need a loader.

Co-authored-by: Cursor <cursoragent@cursor.com>
package.json listed the same superscript file: path twice. Duplicate
JSON keys are last-value-wins so install still worked, but bun.lock
copied the duplication into the workspace deps and packages table.

Co-authored-by: Cursor <cursoragent@cursor.com>

@pullfrog pullfrog 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.

Important

The three-path loader itself is mechanically sound — I verified the repeated-__wbg_init question against the generated wasm-bindgen glue and the jsDelivr URL against a live header check, and both hold. What needs a decision before merge is the retry policy (which now amplifies one failed load into one network fetch per evaluation) and whether an SDK embedded in customers' pages should reach out to a third-party CDN for executable wasm at all.

Reviewed changes — the delta since the prior pullfrog review at 3093af6, i.e. commit ab1d739, plus the npm publish workflow, wasm/tsconfig.json, wasm/README.md / BUILDING.md, and the generated wasm-bindgen --target web glue template.

  • Third load path addedloadCdnModule fetches the exact-version superscript_bg.wasm from jsDelivr and initialises the local --target web glue with it. loadWasmModule's two-level .catch nesting was replaced by a tryLoadPaths() loop over [bundler, inline, cdn] that collects every {path, error} into a failures[] array attached to the thrown Error.
  • Failure memoization removedwasmModulePromise is now cleared in the .catch, so only a successful load is cached.
  • URL override hookcdnWasmUrl() honours globalThis.SUPERWALL_SUPERSCRIPT_WASM_URL when it is a non-empty string.
  • Version codegen — new wasm/scripts/generate-version.ts writes a committed wasm/src/version.ts from package.json, wired into build as generate:version between build:wasm:web and build:ts so the CDN URL pins the exact published version.
  • CHANGELOG — a second 1.0.16 bullet documenting the CDN fallback, the override global, and the memoization change.

Things I checked and found correct, so they don't need defending. Calling the web-target __wbg_init twice on the same imported glue module (inline → cdn) and repeatedly across retries is safe: in the emitted OutputMode::Web template at both 0.2.93 and 0.2.100, the module-level wasm binding is only assigned inside __wbg_finalize_init, which is never reached when __wbg_load rejects — so a failed init leaves wasm === undefined and the next call genuinely re-instantiates with the new module_or_path, while a successful one short-circuits idempotently and runs __wbindgen_start exactly once. jsDelivr serves this file correctly (live check on the published 1.0.3 wasm: content-type: application/wasm, access-control-allow-origin: *; ~1.15 MB, well under the 20 MB single-file limit), the dist/target/web/superscript_bg.wasm path matches what copy:wasm + files: ["dist/"] will actually ship, and VERSION cannot disagree with the published version number — the workflow's collision auto-bump runs before npm publish, and prepublishOnlybuildgenerate:version re-reads the bumped manifest.

⚠️ The /browser entry now fetches and executes wasm from a third-party CDN, unverified and undocumented

loadCdnModule compiles WebAssembly served by cdn.jsdelivr.net with no hash or integrity check — for bytes that are byte-identical to the base64 copy already sitting in the consumer's bundle. For an SDK embedded in customers' pages that is a new third-party runtime dependency, a new supply-chain surface, and a request disclosing the end user's IP and exact SDK version to jsDelivr, none of which the consumer opted into. The only escape hatch, globalThis.SUPERWALL_SUPERSCRIPT_WASM_URL, is mentioned in CHANGELOG.md and nowhere else: wasm/README.md has no browser-entry or CSP section at all.

Technical details
# Third-party CDN wasm execution is opt-out, unverified, and undocumented

## Affected sites
- `wasm/src/browser.ts:9-18``cdnWasmUrl()` hardcodes `cdn.jsdelivr.net` as the default; the
  `SUPERWALL_SUPERSCRIPT_WASM_URL` override must be set on `globalThis` before the first
  `evaluateWithContext` call, which is a hard ordering constraint for a consumer to discover
  from a CHANGELOG line.
- `wasm/src/browser.ts:65-71` — the fetched response goes straight into
  `glue.default({ module_or_path: … })``WebAssembly.instantiateStreaming`. No SHA-256, no
  SRI, no comparison against the base64 copy the package already carries.
- `wasm/src/browser.ts:83` — registered unconditionally as the third path, so it is reachable
  in every consumer's production build, not behind a flag.
- `wasm/README.md` — no mention of the `/browser` entry's network behaviour, the required
  `connect-src https://cdn.jsdelivr.net` CSP allowance, or the override global.

## Required outcome
- The CDN fetch is an explicit, documented product decision rather than an implicit default,
  and a consumer can predict and disable it without reading the source.

## Suggested approach (optional)
- Decide opt-in vs opt-out. Note the marginal coverage is narrow: `loadCdnModule` shares the
  `import('../target/web/superscript.js')` glue with `loadInlineModule`, so it can only fire
  when the glue loaded but the base64 sibling module did not (or `atob`/decode failed) — a
  much smaller set than the file-asset bundler group the inline path already covers.
- If it stays on by default, document it in `wasm/README.md`: the origin, the CSP allowance,
  and `SUPERWALL_SUPERSCRIPT_WASM_URL` (including that it must be set before first use).
- Consider verifying the fetched bytes before instantiating — `scripts/inline-wasm.ts`
  already reads the binary, so it can emit a SHA-256 alongside `wasmBase64` at no extra
  build cost.
- Consider a Superwall-controlled origin instead of a public multi-tenant CDN.

## Open questions for the human
- Is a silent third-party request from inside customers' pages acceptable for this SDK, or
  should the last-resort path be explicitly enabled by the integrator?
- What is the intended story for enterprise consumers with a strict `connect-src`? Today they
  get three failed paths and, per the inline note on `loadWasmModule`, a retry on every
  evaluation.
- Secondary: `cdnWasmUrl()` 404s for anyone building `wasm/` from a git checkout whose version
  was never published — the publish workflow's auto-bump is never committed back, so git and
  npm versions drift by design. Harmless today (such a build has working local paths) but it
  means the last-resort path is not universally available.

ℹ️ Nitpicks

  • CHANGELOG.md:9-10 — the 1.0.16 notes carry two bullets describing the same loader: the first says it "tries the bundler target first and, if that import fails, falls back to … base64-inlined wasm bytes", the second bolts a third path on. A reader of the release sees two changes where there is one three-path loader. Worth folding into a single bullet before merge.
  • The untested-loader gap from the review on f029916 has widened rather than narrowed: ab1d739 adds a second unexercised runtime path plus a codegen step, and nothing asserts that the generated VERSION matches the tarball or that the CDN URL shape resolves. A single post-build script that atob-decodes the inline module, initialises the web glue and evaluates one known expression would cover the whole new surface without a bundler.
  • wasm/package.json:35generate:version rewrites the tracked wasm/src/version.ts as a side effect of npm run build, so a build on a branch whose package.json version differs leaves the working tree dirty. Not wrong, just surprising for a file that is both generated and committed.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread wasm/src/browser.ts
Clearing the memo on every failure re-ran all three paths per
evaluateWithContext: a fresh jsDelivr fetch plus atob of the ~1.5 M-char
blob. Fail-fast for 10s after a total miss so a CSP/offline page does not
issue N network requests for N audience evals; after the cooldown a
single retry is allowed so a blip can recover. Successful inline base64
decode is cached independently of glue init, so a retry does not redo
atob.

Co-authored-by: Cursor <cursoragent@cursor.com>

@pullfrog pullfrog 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.

ℹ️ No critical issues — two small cleanups inline.

Reviewed changes — the delta since the prior pullfrog review at 2731402, i.e. commit a263429, plus the surrounding wasm/src/browser.ts state machine and the check-run status on the new head.

  • Bounded the retry rateloadWasmModule now records a total failure in lastFailure = { error, at } and replays Promise.reject(lastFailure.error) for RETRY_COOLDOWN_MS = 10_000 before allowing exactly one fresh attempt, replacing the unconditional memo-clear that turned every evaluateWithContext call into a full three-path retry.
  • Cached the inline decode independently of init — new loadInlineWasmBytes() memoizes the atobUint8Array decode in inlineWasmBytes / inlineWasmBytesPromise, self-nulling the promise in a .catch so a missing or mangled inline chunk is re-imported on a later retry rather than cached as a permanent rejection.
  • Rewrote the second CHANGELOG.md 1.0.16 bullet to describe the cooldown and the decode cache instead of the removed memoization.

I audited the new module-level state machine for the failure modes this shape usually has, and it holds. loadWasmModule is a plain synchronous function with no await before it assigns wasmModulePromise, so two concurrent evaluateWithContext calls cannot both observe null and both start a load; the cooldown check sits after the if (wasmModulePromise) early return, so an in-flight post-cooldown retry is shared rather than duplicated; the rejection handler's wasmModulePromise = null cannot null a newer promise, since it is the only nuller and nothing reassigns while non-null; and ??= in loadInlineWasmBytes completes before its own .catch can run, because promise callbacks are always asynchronous. Every path is awaited at the single call site, so no rejection escapes unhandled. The retry loop also doesn't have a bandwidth pathology: jsDelivr serves the wasm with a long-lived immutable cache-control, so a "fetch succeeds, instantiate fails" retry hits the HTTP cache, and a CSP-blocked or offline fetch transfers nothing. build-test-publish is green on a263429, which also confirms tsc accepts the new code.

Two items from earlier reviews are unchanged by this commit and still need a human: the opt-in-vs-opt-out decision on the default jsDelivr fetch, and the absence of any CI that executes the fallback paths — which this commit widens slightly, since the cooldown is time-dependent module state that nothing exercises.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread wasm/src/browser.ts Outdated
Comment thread wasm/src/browser.ts
ianrumac and others added 2 commits September 8, 2026 16:44
…n.ts

Release notes described one three-path loader as two bullets; fold them.
Add a post-build script that checks generated VERSION against package.json,
asserts the jsDelivr URL shape, byte-compares the inline module to
superscript_bg.wasm, then inits the web glue and evaluates a known
expression. gitignore src/version.ts so npm run build does not dirty the
tree when package.json's version differs from a committed copy.

Co-authored-by: Cursor <cursoragent@cursor.com>
Move the web-target docblock back onto loadInlineModule and give the
decode helper its own note. After glue.default resolves the overall load
is memoized, so keeping the ~1.15 MB Uint8Array for the page lifetime
was wasted; null the promise so it can be collected. Failures still
keep the decode for a cooldown retry.

Co-authored-by: Cursor <cursoragent@cursor.com>

@pullfrog pullfrog 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.

ℹ️ No critical issues — the new post-build check is real coverage; two small cleanups inline.

Reviewed changes — the delta since the prior pullfrog review at a263429, i.e. commit c554319, plus build_wasm.sh, both npm workflows, wasm/tsconfig.json, and the two review threads still open on wasm/src/browser.ts.

  • Post-build loader check added — new wasm/scripts/test-browser-loader.ts, wired as test:browser-loader at the end of build, so it runs both in CI (build_wasm.shbun run build) and in prepublishOnly. It byte-compares atob(wasmBase64) against dist/target/web/superscript_bg.wasm, initialises the --target web glue from those bytes, and asserts a real expression evaluates to {"Ok":{"type":"bool","value":true}}.
  • wasm/src/version.ts untracked — added to .gitignore and deleted from the index, with generate-version.ts's header comment reversed to match; build still runs generate:version before build:ts.
  • CHANGELOG.md 1.0.16 folded to one bullet describing the three-path loader, the cooldown, and the build-time-vs-runtime scope note in a single entry.

wasm/src/browser.ts is untouched by this commit, which I confirmed against the commit range rather than inferring it from the diff — so the loader state machine audited on a263429 still stands, and the two open threads on that file are unchanged rather than addressed. build-test-publish is green on c554319, which is meaningful here beyond tsc: it means the new check genuinely executed the base64 decode and a wasm instantiation on a Linux runner.

Two items from earlier reviews still need a human and are unchanged: the opt-in-vs-opt-out decision on the default jsDelivr fetch (plus its absence from wasm/README.md), and the SUPERWALL_SUPERSCRIPT_WASM_URL override being documented only in the changelog.

ℹ️ The new check covers the build artifacts, not the /browser loader it is named after

test-browser-loader.ts never imports dist/esm/browser.js. It re-implements the load sequence against the raw wasm-pack output, so tryLoadPaths, loadWasmModule's cooldown, loadInlineModule, loadCdnModule, cdnWasmUrl and the exported evaluateWithContext are all still unexecuted. Deleting loadInlineModule from the fallback array, or breaking evaluateWithContext's wrapper outright, leaves the check green. Worth noting this is the exact shape the review on f029916 asked for, so it is an upgrade path rather than a miss — but a script that drove the real export would cover strictly more for roughly the same line count.

Technical details
# Post-build check does not execute `wasm/src/browser.ts`

## Affected sites
- `wasm/scripts/test-browser-loader.ts:33-96` — imports `dist/target/web/superscript.js` and
  `dist/target/web/superscript_bg_inline.js` directly, performs its own `atob``Uint8Array``glue.default({ module_or_path })` sequence, and calls the raw `glue.evaluate_with_context`
  with a hand-built host object.
- `wasm/src/browser.ts:74-81` (`loadInlineModule`), `:90-96` (`loadCdnModule`), `:103-123`
  (`tryLoadPaths`), `:125-149` (`loadWasmModule`), `:151-169` (`evaluateWithContext`) — none
  are reachable from the check.

## Required outcome
- A failure in `browser.ts`'s own loader orchestration — wrong fallback ordering, a missing
  path, a wrong `module_or_path` key, a broken `evaluateWithContext` wrapper — fails the build.

## Suggested approach (optional)
- Import the compiled entry instead of the artifacts: `await import(fileUrl('dist/esm/browser.js'))`
  and call the exported `evaluateWithContext(input, context)`. From `dist/esm/`, the relative
  `../target/browser/superscript.js` and `../target/web/*` specifiers both resolve after
  `copy:wasm`, and under a plain JS runtime `loadBundlerModule()` fails on the `.wasm` import,
  so `tryLoadPaths` genuinely walks to the inline path. Keep the existing byte-comparison
  block — it covers `inline-wasm.ts`, which the entry point cannot.
- Calling `evaluateWithContext` twice and asserting the second call does not re-instantiate
  would cover the memoization the changelog advertises.

## Open questions for the human
- Is `loadCdnModule` intended to stay untestable in CI, or is a fixture-served
  `SUPERWALL_SUPERSCRIPT_WASM_URL` (pointing at the local `dist/target/web/superscript_bg.wasm`)
  worth the extra moving part?

ℹ️ Nitpicks

  • .gitignore/target and /wasm/dist were dropped alongside the new wasm/src/version.ts entry. Both are still ignored by later rules (git check-ignore -v resolves wasm/target/… to .gitignore:133 target/ and wasm/dist/… to .gitignore:149 **/dist/**), so it is a no-op — flagging only in case the deletion was collateral.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread wasm/scripts/test-browser-loader.ts Outdated
Comment thread .gitignore
The CDN path is no longer registered by default — customers' pages
must not silently request a third-party origin. Opt in before the first
evaluateWithContext: SUPERWALL_SUPERSCRIPT_WASM_CDN = true for the
pinned jsDelivr URL, or SUPERWALL_SUPERSCRIPT_WASM_URL for a
self-hosted copy (wins if both are set). wasm/README.md covers entries,
CSP connect-src, and the ordering constraint.

Co-authored-by: Cursor <cursoragent@cursor.com>

@pullfrog pullfrog 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.

ℹ️ No critical issues — the opt-in switch closes the CDN question cleanly; one doc nit inline.

Reviewed changes — the delta since the prior pullfrog review at c554319, i.e. commits 731d09f and edff047 (git diff --stat c554319..edff047 = CHANGELOG.md, wasm/README.md, wasm/src/browser.ts), plus wasm/README.md in full, wasm/package.json's exports, and the four review threads standing on the PR.

  • Dropped the inline decode cache after a successful init731d09f deletes the inlineWasmBytes variable outright (the promise was already the only cache that mattered) and nulls inlineWasmBytesPromise once glue.default resolves, so the decoded ~1.15 MB Uint8Array is collectable in exactly the Bun/esbuild consumers this fix targets. The same commit moves the two doc blocks back onto the functions they describe.
  • Made the URL fetch opt-inedff047 turns cdnWasmUrl() into string | null: a non-empty SUPERWALL_SUPERSCRIPT_WASM_URL wins, otherwise SUPERWALL_SUPERSCRIPT_WASM_CDN === true yields the version-pinned jsDelivr URL, otherwise null. tryLoadPaths builds its paths array explicitly and only pushes the third entry when a URL exists, and loadCdnModule(url) takes the URL as a parameter rather than resolving it internally.
  • Documented the browser entry — new ## Entries and ## Optional CDN fallback sections in wasm/README.md (which npm ships regardless of files), covering both globals, the precedence rule, the jsDelivr URL shape and the connect-src allowance; the single CHANGELOG.md 1.0.16 bullet was rewritten to match.

Both threads from the a263429 review are genuinely addressed by 731d09f — replied to and resolved. The opt-in switch resolves the ⚠️ section from the ab1d739 review: the third-party fetch is now off by default, so there is no unannounced request from customers' pages and no CSP change for anyone who doesn't ask for one, and byte verification becomes the opting-in integrator's own call rather than a default the SDK imposes. I checked every consumer-facing claim in the new README and the changelog bullet against the code, and all of them hold except the ordering sentence flagged inline: the no-network default (nothing reachable fetches when both globals are unset — the inline path hands the glue a BufferSource, which bypasses its URL branch), the precedence rule, the strict-boolean _CDN check, the decode-cache lifetime, the 10s cooldown with one post-cooldown attempt, the URL shape against what copy:wasm + files: ["dist/"] publish, and the CSP note. build-test-publish is green on edff047, so tsc accepts the explicitly-typed paths array that replaced the as const tuple, and test:browser-loader ran on a Linux runner.

Two threads from the c554319 review are untouched by this delta and still need the author: the untracked wasm/src/version.ts breaking bare tsc / build:ts / editor TS servers on a fresh checkout, and the tautological CDN-URL assertion in test-browser-loader.ts. Nothing in this delta narrows the loader-coverage gap either — cdnWasmUrl's new gating branch is one more piece of browser.ts that no test executes — but that is already on record and not a new finding.

ℹ️ Nitpicks

  • wasm/src/browser.ts:133-137 — the aggregated all wasm load paths failed message now enumerates only the paths that were actually attempted, which by default is the two local ones. A field report from a consumer whose inline chunk got mangled therefore carries no hint that SUPERWALL_SUPERSCRIPT_WASM_CDN / SUPERWALL_SUPERSCRIPT_WASM_URL exist; one trailing clause naming them would make the error self-describing, which is the property the rest of this error was built for.
  • wasm/README.md:33 (pre-existing, newly adjacent) — the Setup snippet still says import * as wasm from "@superwall/superscript", but wasm/package.json's exports has only ./node and ./browser, so that specifier throws ERR_PACKAGE_PATH_NOT_EXPORTED. It now sits ~20 lines under a new ## Entries list that gives the correct subpaths, which makes the contradiction hard to miss.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread wasm/README.md Outdated
The aggregated failure only listed attempted paths, so a mangled-inline
report had no pointer at SUPERWALL_SUPERSCRIPT_WASM_CDN / _URL. Append
that hint when the CDN path was skipped. README Setup imported the
package root which is not an exports target; use /browser and /node.
Phrase the opt-in gate around a load attempt, not the first evaluate:
flags are re-read on each attempt, including the post-cooldown retry.

Co-authored-by: Cursor <cursoragent@cursor.com>

@pullfrog pullfrog 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.

ℹ️ No critical issues — both nits from the last review are addressed; one follow-on doc gap inline.

Reviewed changes — the delta since the prior pullfrog review at edff047, i.e. commit ff720ec (git diff --stat edff047..ff720ec = CHANGELOG.md 1 line, wasm/README.md 14, wasm/src/browser.ts 19), plus wasm/README.md in full, the /browser and /node export surfaces, and the three review threads standing on the PR.

  • Load-timing wording now matches the codewasm/README.md:15, cdnWasmUrl()'s doc block and the CHANGELOG.md 1.0.16 bullet all describe the opt-in gate as "before a load attempt", re-read at the start of each fresh attempt including the post-cooldown retry, and never re-read after a success. That is exactly what tryLoadPaths does — cdnWasmUrl() is called per invocation and loadWasmModule only re-enters it once the 10s window has elapsed.
  • The aggregate failure error names the opt-intryLoadPaths splits the message into the unchanged detail join over failures plus a hint that is non-empty only when cdnUrl === null, so a field report from a consumer whose two local paths both failed now says which globals to set. The failures own-property assignment is untouched, and the default (no flags set) still issues no network request.
  • Setup snippet points at a real entry — the import * as wasm from "@superwall/superscript" line, which would have thrown ERR_PACKAGE_PATH_NOT_EXPORTED against an exports map with only ./node and ./browser, is replaced by a named import from /browser with a /node variant in a comment, and the prose below drops the wasm. prefix. evaluateWithContext is a named export of both src/browser.ts:175 and src/node.ts:22, and the existing main() example already called it bare, so the page is now internally consistent.

No control flow changed in this delta, so the loader state machine audited on a263429 and the opt-in gating audited on edff047 both still stand. build-test-publish is green on ff720ec, which means tsc accepts the new detail/hint split and test:browser-loader ran on a Linux runner.

The two threads from the c554319 review are untouched by this delta and still need the author: the untracked wasm/src/version.ts breaking bare tsc / build:ts / editor TS servers on a fresh checkout, and the tautological CDN-URL assertion in test-browser-loader.ts.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread wasm/README.md Outdated
The post-build URL check reconstructed the jsDelivr path from package.json
and compared it to itself. Call the exported cdnWasmUrl() instead: null
when unset, pinned jsDelivr when SUPERWALL_SUPERSCRIPT_WASM_CDN is true,
URL override wins. Chain generate:version into build:ts so a fresh clone
can tsc without a prior full wasm build.

Co-authored-by: Cursor <cursoragent@cursor.com>

@pullfrog pullfrog 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.

ℹ️ No new issues — the two threads from the c554319 review are properly closed; one nit inline.

Reviewed changes — the delta since the prior pullfrog review at ff720ec, i.e. commit 2c7a632 (git diff --stat ff720ec..2c7a632 = .gitignore 3 lines, wasm/package.json 2, wasm/scripts/generate-version.ts doc-only, wasm/scripts/test-browser-loader.ts +28/-5, wasm/src/browser.ts +2), plus wasm/tsconfig.json, the full wasm/package.json script graph, and the three review threads standing on the PR.

  • Version codegen chained into build:ts"build:ts": "npm run generate:version && npm run build:ts:esm && npm run build:ts:cjs", with the now-redundant generate:version dropped from the build chain. npm run build:ts is self-sufficient on a fresh checkout where the gitignored wasm/src/version.ts does not yet exist; a bare tsc or an editor TS server still needs one manual generate:version, which generate-version.ts:2-5 now says explicitly.
  • The CDN-URL check is no longer a tautologytest-browser-loader.ts:28-53 imports the real cdnWasmUrl() from dist/esm/browser.js and asserts three gating outcomes: null with neither flag set, the version-pinned jsDelivr URL under SUPERWALL_SUPERSCRIPT_WASM_CDN = true, and SUPERWALL_SUPERSCRIPT_WASM_URL winning when both are set. All three can genuinely fail — the expected values are built independently of the function under test (browser.ts:26 hardcodes @superwall/superscript while the test derives it from pkg.name, and the override case compares against a literal), so a host, path-segment or precedence edit now breaks the build. Both globals are deleted before the glue-init block, so the byte-comparison and evaluation below still run with no network involved.
  • cdnWasmUrl exported from /browserbrowser.ts:201, which is what makes the above possible. No runtime consequence: tryLoadPaths:124 already referenced it, so nothing changes for tree-shaking or the default no-network behaviour.

build-test-publish is green on 2c7a632, which is load-bearing for this delta beyond tsc: it means await import('dist/esm/browser.js') actually resolves and evaluates under Bun despite the extensionless ./version specifier tsc emits, and that the reordered generate:version still runs before both tsc passes. The loader state machine audited on a263429 and the opt-in gating audited on edff047 are untouched here.

This is also the first commit in the PR that executes any of wasm/src/browser.ts in CI. The remainder — tryLoadPaths ordering, the RETRY_COOLDOWN_MS window, loadInlineModule, loadCdnModule, and the exported evaluateWithContext — is still unexercised, but the blocker named in the c554319 review is now gone: the script already imports the compiled entry, so driving evaluateWithContext(input, host) through it is a few lines rather than a new harness. Noting it as status, not as a new finding.

One thread from the previous review is untouched by this delta and still needs the author: wasm/README.md:35 — the Setup snippet imports only evaluateWithContext from /browser, while the snippets below it use SuperscriptHostContext and PassableValue, neither of which /browser exports.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread wasm/src/browser.ts

@pullfrog pullfrog 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.

ℹ️ No new issues — the README type-surface thread is closed; the one nit from the previous review is still open.

Reviewed changes — the delta since the prior pullfrog review at 2c7a632, i.e. commit a8560fa (git diff 2c7a632..a8560fa = wasm/README.md +11/-6, wasm/src/browser.ts +1), plus the /browser and /node export surfaces, wasm/src/types.ts, and the two review threads standing on the PR.

  • PassableValue re-exported from /browserwasm/src/browser.ts:198 adds it to the export type { … } from './types' block, so /browser and /node now agree on the four names the README uses. No conflict with the local import type { SuperscriptHostContext, ExecutionContext } at :1, since a re-export creates no local binding, and PassableValue is a plain export interface in types.ts:3.
  • Setup snippet now compiles as writtenwasm/README.md:35-42 imports evaluateWithContext plus type ExecutionContext / type PassableValue / type WasmHostContext from /browser, and :53 switches the example class to implements WasmHostContext, the alias the entry actually exports. Every one of those four names is used further down (:53, :54, :100, :133), and the snippet's computed_property(name: string, args: [PassableValue]): PassableValue signature is character-identical to SuperscriptHostContext in types.ts, so pasting the page into a .ts file type-checks rather than merely resolving.

No control flow changed in this delta, so the loader state machine audited on a263429 and the opt-in gating audited on edff047 both still stand. The thread on wasm/README.md:35 from the ff720ec review is genuinely addressed and has been resolved. Checks were still running on a8560fa when this review was written; for a doc change plus a type-only re-export the only thing they add is tsc acceptance of the widened export type list.

One thread is untouched by this delta and still needs the author: wasm/src/browser.ts:202cdnWasmUrl became public API in 2c7a632 and is still absent from wasm/README.md's ## Optional CDN fallback section.

Pullfrog  | Fix it ➔View workflow run | Using Claude Opus𝕏

@ianrumac
ianrumac merged commit 2466696 into master Sep 9, 2026
3 checks passed
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.

1 participant