test(runtime): guard tsx/esm→esbuild transform path on boot (#5757)#5773
Merged
Merged
Conversation
#5757 reported that a fresh `npm install omniroute` pulls `esbuild@0.28.1` transitively via `tsx` (a runtime dependency the CLI registers at boot in `bin/omniroute.mjs`), and proposed forcing `esbuild@0.27.4`. That override is unsafe: `tsx@4.22.4` requires `esbuild@~0.28.0` and `fumadocs-mdx@15` (also a runtime dep) requires `esbuild@^0.28.0`; forcing 0.27.x pushes esbuild below both, and 0.28.1 is currently the latest release. The reported transform failure also does not reproduce — OmniRoute targets ES2022, its minimum supported Node is 22.2 (destructuring is native), and tsx targets the running Node, so esbuild never lowers to an unsupported target. Instead of an unsafe version pin, add two regression guards: - functional: spawn the real `node --import tsx/esm` loader on a fixture packed with modern syntax (destructuring/spread, class+private fields, optional chaining, nullish, logical assignment, async + top-level await) and assert it transforms + runs correctly. Fails if a future esbuild regresses the boot path. - dependency-shape: assert the resolved esbuild stays within tsx's declared range, so nobody reintroduces the out-of-range override this issue proposed. No production code changed; no esbuild version pinned.
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses #5757 — a fresh
npm install omniroute@3.8.42pullsesbuild@0.28.1transitively viatsx, and the reporter proposed forcingesbuild@0.27.4.Why not the proposed override
overrides: { esbuild: "0.27.4" }is unsafe here:tsx@4.22.4declaresesbuild@~0.28.0andfumadocs-mdx@15(also a runtimedependency) declaresesbuild@^0.28.0. Forcing0.27.xpushes esbuild below both ranges.esbuild@0.28.1is currently the latest release — there is no fixed0.28.2to pin up to.Why the failure doesn't reproduce
The quoted error ("Transforming destructuring to the configured target environment is not supported yet") only fires when esbuild lowers to a pre-ES2015 target. In OmniRoute that path can't be reached on a supported runtime:
tsconfig targetis ES2022 (destructuring is native → no lowering).bin/nodeRuntimeSupport.mjs) — all support destructuring.tsxtargets the running Node, not the tsconfig downlevel target (verified empirically: even a hostiletarget: es5tsconfig transforms cleanly viatsx/esmon Node 24 with esbuild@0.28.1).What this PR adds (a safe guard instead of a version pin)
tests/unit/tsx-runtime-transform-5757.test.ts+tests/unit/_fixtures/tsx-runtime-modern-syntax.ts:node --import tsx/esmloader (exactly whatbin/omniroute.mjsregisters at boot) on a fixture packed with modern syntax (destructuring/spread, class + private fields, optional chaining, nullish, logical assignment, async + top-level await) and asserts it transforms + runs correctly. Fails loudly if a future esbuild regresses the boot path.dependencies.esbuild), so nobody reintroduces the out-of-range override this issue proposed. Proven to catch0.27.x.No production code changed; no esbuild version pinned.
Validation
node --import tsx/esm --test …→ 2/2), and both proven to have teeth (dependency guard rejects0.27.4/0.27.7; functional guard fails on non-zero child status).typecheck:coreclean · lint 0 errors · the_fixtures/file is not matched by thetest:unitglob.Refs #5757 (kept open pending the reporter's actual error log / exact repro command — see issue comment).