Repository navigation
Description
Instrument code that guards its DOM access with typeof window !== 'undefined' still throws when the API evaluates the bundle, so the instrument cannot be uploaded or imported. Where the host has no DOM, createBundle's GLOBALS preamble shadows document, self and window with Proxy objects that throw on any property access. They exist to give a better error message, but a Proxy is an object, so typeof window is 'object' on the server. The standard guard then passes, and the guarded window.… access hits the throwing trap. The UMD-style typeof self !== 'undefined' ? self : this check found in many copied-in libraries fails the same way. InstrumentsService.create evaluates every bundle in a Node vm with no DOM, so such an instrument is rejected with Failed to interpret instrument bundle (422), or skipped during a repository import, even though it is correct browser code.
Where
packages/instrument-bundler/src/bundle.ts:19-35:
const __createProxy = (name) => {
// ...
return new Proxy({ name }, {
get(target, property) {
throw new Error(formatErrorMessage('get', property.toString(), target.name))
},
// ...
};
const document = globalThis.document ?? __createProxy('document');
const self = globalThis.self ?? __createProxy('self');
const window = globalThis.window ?? __createProxy('window');Reproduce
- Bundle a form whose
index.tshas, at module scope,const lang = typeof window !== 'undefined' ? window.navigator.language : 'en';andexport default { kind: 'FORM', lang, … }. - Evaluate the bundle the way the API does:
vm.runInContext(bundle, vm.createContext({})), or upload it throughPOST /v1/instruments.
Actual: Error: Cannot get property 'navigator' of object 'window' in global scope of file 'index.ts' (verified on main with vm), and the upload fails with 422.
Expected: the guard sees no window on the server, the bundle evaluates, and the instrument is stored. Unguarded DOM access at module scope still fails with a message naming the file.
Tests
packages/instrument-bundler/src/__tests__/bundle.test.ts: it('should let a typeof window guard skip DOM access when evaluated without a DOM, so guarded instruments can be uploaded'). Evaluate the bundled string with vm.runInContext in an empty context.
e2e, in testing/src/specs/admin-instruments.spec.ts (or wherever the upload flow is covered): uploading an instrument with a guarded module-scope window access succeeds.
Suggested fix
Drop the shadowing proxies, so document, self and window are truly undeclared when the host has none. Keep the helpful message another way: wrap the evaluation in createBundle's IIFE in a try/catch that rethrows a ReferenceError for document, self or window with globalThis.__ODC_BUNDLER_ERROR_CONTEXT attached, so unguarded access still names the file.
Activity
joshunrau commentedon Oct 26, 2026
Still reproduces on
main(d86740e). No commit since this was filed touchesbundle.tsorplugin.ts, and no open PR touchesbundle.ts.The same
consts also break bundlingA top-level binding named
document,selforwindowin any input collides with the preamble's declarations (bundle.ts:33-35).bundle()then throwsThe symbol "document" has already been declaredfrom theesbuild.transformincreateBundle(bundle.ts:60). It is a plain esbuild error, not anInstrumentBundlerError, located at a line of the synthetic wrapper (<stdin>:30here), so it names neither the author's file nor their line, and the playground shows noCodeErrorBlock. It fails whereverbundle()runs: the playground (esbuild-wasm 0.28.2 reports the same error), repository imports and the CLI.Reproduced with
bundle({ inputs: [{ name: 'index.ts', content: "const document = 'x'; export default { kind: 'FORM', document };" }] }). A top-levelfunction window() {}, orexport const selfin a second file, fails the same way. Bindings inside a function body are fine. Dropping the declarations fixes this too, and it needs its own test.On the suggested fix
Dropping the proxies is right. Three details for the
catch:error instanceof ReferenceErrorrather than matching the identifier in the message, whose wording differs by engine (Safari saysCan't find variable: x). The bundle throws and catches in its own realm, soinstanceofholds in the vm context and in the browser. Catching everyReferenceErroralso names the file fornavigator,localStorageand similar globals, which the proxies never covered: an unguardednavigator.languagefails today with a barenavigator is not defined.new ReferenceError(error.message + " in global scope of file '" + context + "'", { cause: error }), and rethrow everything else unchanged.InstrumentInterpreteralready wraps evaluation errors (packages/instrument-interpreter/src/index.ts:28), andInstrumentErrorFallbackshows one level ofcause(packages/react-core/src/components/InstrumentErrorFallback/InstrumentErrorFallback.tsx:33-35), so a generic wrapper message would push the real one out of view.trymust enclose${inject}andreturn __exportsas well as${output.js}. ThecreateBundletests passconst __exports(packages/instrument-bundler/src/__tests__/bundle.test.ts:8), which areturnafter the block could not see.Checked with esbuild 0.28.2, the same transform options and an empty
vmcontext: this shape evaluates the guarded case, letstypeof self !== 'undefined' ? self : globalThisfall through, rejectswindow.innerWidthwithwindow is not defined in global scope of file 'index.ts', and passes aTypeErrorthrough unchanged.packages/instrument-bundler/AGENTS.md:48-56and.agents/docs/architecture/instrument-pipeline.md:83-86describe the proxies and need updating in the same commit.Tests
Unit, in
bundle.test.ts: the issue's case, an unguarded access, aTypeErrorpassing through, andbundle()accepting a top-leveldocument. Assert onerror.name, because the error comes from the vm realm and fails an outerinstanceof ReferenceError. Write the sources without/runtime/v1imports, unlike therepositories/fixtures: the bundle turns those into a dynamicimport(), which an empty vm context rejects withERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING.e2e: the suite cannot build a fresh bundle. Its only
POST /v1/instrumentsre-posts a seeded one (testing/src/specs/authorization.spec.ts:402-405, throughreadSeededBundleat:278-284). A hand-written bundle string is enough for #1712 and #1706, but tests nothing here, because the preamble is what changes. Two options:testing/src/specs/playground.spec.ts: open a share link with the guarded source, log in with the API base URL pointed at the test API, and upload through Upload Bundle, expecting the201success notification. No new dependency, and one test covers the browser bundler and the API evaluation. PutuniqueIdininternal.name, since a repeated name and edition is a 409. build: upgrade dependencies held back by breaking changes #1660, fix(playground): search instruments by label #1817 and fix: resolve 26 low-difficulty issues, one commit each #1825 also edit this spec.testing/src/specs/admin-instruments.spec.ts: callbundle()in the test and POST the result withadminToken. Shorter, and Instrument upload validation ignores the instrument's language, accepting untranslated bilingual instruments and rejecting language: ['en'] #1712 and FILE instrument schema accepts file groups that can never be completed: min above max, or two groups sharing a basename #1706 could build their bundles the same way. But it adds@opendatacapture/instrument-bundler, and esbuild with it, totesting/package.json, a new dependency that needs agreeing first.I would take option 1.
Rollout: repository imports bundle inside the API (
apps/api/src/instrument-repos/instrument-repos.service.ts:280), so an API redeploy covers them. Playground uploads need a playground redeploy, and CLI users a release after 2.5.2 and a rebuild. Stored bundles need nothing: each one has already evaluated on the server.Triage note written with Claude Code. Code references point at
main@d86740e.