-
-
Notifications
You must be signed in to change notification settings - Fork 974
fix(core): vendor superjson to fix CJS compatibility issue #2940
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This fixes the ERR_REQUIRE_ESM error that occurs when using @trigger.dev/core in CJS environments with Node.js versions < 22.12. The superjson package (v2.x) is ESM-only, but Node.js doesn't support require() of ESM modules in older versions. The fix bundles superjson@2.2.1 using esbuild into both CJS and ESM formats, which are then imported by the existing wrapper modules. This ensures the package works correctly in all environments without relying on Node.js's experimental require(ESM) feature. Fixes #2937 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 3375d3d The changes in this PR will be included in the next version bump. This PR includes changesets to release 27 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Comment |
| var isURL = (payload) => payload instanceof URL; | ||
|
|
||
| // ../../node_modules/.pnpm/superjson@2.2.1/node_modules/superjson/dist/pathstringifier.js | ||
| var escapeKey = (key) => key.replace(/\./g, "\\."); |
Check failure
Code scanning / CodeQL
Incomplete string escaping or encoding High library
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 5 hours ago
In general, to fix this class of problems you must escape the escape character itself before (or while) escaping other meta-characters, and ensure that the parser correctly inverts the escaping. Here, that means encoding backslashes in a reversible way before turning dots into \.. The decoder (parsePath) then needs to understand both the escaped dots and escaped backslashes.
The best minimal fix without changing overall functionality is:
- Change
escapeKeyso that it first escapes existing backslashes (e.g., replace\with\\globally), then escapes dots by turning.into\.. This ensures that any\.seen by the parser must have been produced by the escaping step, not from an original backslash+dot in the key. - Update
parsePathso that:- It recognizes
\.as an escaped dot (current behavior). - It also recognizes
\\as an escaped backslash and appends a single\to the segment. - Other backslashes are treated literally.
- It recognizes
To keep the change minimal and localized, we only modify the escapeKey function and the loop in parsePath in packages/core/src/v3/vendor/superjson.cjs. No new imports or external libraries are needed; we can implement this with built-in string methods and simple control flow.
Concretely:
- At line 195, replace
var escapeKey = (key) => key.replace(/\./g, "\\.");with a version that first escapes backslashes:var escapeKey = (key) => key.replace(/\\/g, "\\\\").replace(/\./g, "\\."); - In
parsePath, adjust the character loop so it checks for\\(escaped backslash) and\.(escaped dot) in order, handles them appropriately, and only treats an unescaped.as a segment separator.
-
Copy modified line R195 -
Copy modified lines R202-R207
| @@ -192,13 +192,19 @@ | ||
| var isURL = (payload) => payload instanceof URL; | ||
|
|
||
| // ../../node_modules/.pnpm/superjson@2.2.1/node_modules/superjson/dist/pathstringifier.js | ||
| var escapeKey = (key) => key.replace(/\./g, "\\."); | ||
| var escapeKey = (key) => key.replace(/\\/g, "\\\\").replace(/\./g, "\\."); | ||
| var stringifyPath = (path) => path.map(String).map(escapeKey).join("."); | ||
| var parsePath = (string) => { | ||
| const result = []; | ||
| let segment = ""; | ||
| for (let i = 0; i < string.length; i++) { | ||
| let char = string.charAt(i); | ||
| const isEscapedBackslash = char === "\\" && string.charAt(i + 1) === "\\"; | ||
| if (isEscapedBackslash) { | ||
| segment += "\\"; | ||
| i++; | ||
| continue; | ||
| } | ||
| const isEscapedDot = char === "\\" && string.charAt(i + 1) === "."; | ||
| if (isEscapedDot) { | ||
| segment += "."; |
| var isURL = (payload) => payload instanceof URL; | ||
|
|
||
| // ../../node_modules/.pnpm/superjson@2.2.1/node_modules/superjson/dist/pathstringifier.js | ||
| var escapeKey = (key) => key.replace(/\./g, "\\."); |
Check failure
Code scanning / CodeQL
Incomplete string escaping or encoding High library
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 5 hours ago
In general, when designing a custom escaping scheme that uses backslash as the escape character, you must first escape existing backslashes before escaping other characters (such as .) so that decoding can correctly distinguish literal backslashes from escapes. Here, escapeKey currently only escapes dots by turning . into \. but never doubles existing backslashes. parsePath, however, interprets \. specially as an escaped dot and does not treat \\ as a literal backslash escape, so a key with backslashes can be misparsed.
The best fix, without changing external behavior for existing “simple” keys, is to extend both the encoder (escapeKey) and decoder (parsePath) to handle backslashes in a symmetric way. A straightforward scheme is:
- In
escapeKey, first escape backslashes by replacing each\with\\, then escape dots by replacing.with\.. This ensures any sequence that looks like an escape in the encoded string was deliberately produced byescapeKey. - In
parsePath, interpret\\as a literal backslash and\.as a literal dot, and treat any other backslash as a literal backslash followed by the next character. This makes decoding robust and preserves round-tripping even if other characters follow a backslash.
Concretely:
- Modify line 161 to add a backslash-escaping step before the existing dot-escaping.
- Adjust the loop in
parsePath(lines 166–181) so it can decode\\and\.correctly, rather than only handling\.. This change is fully local topackages/core/src/v3/vendor/superjson.jsand requires no new imports or external dependencies.
-
Copy modified line R161 -
Copy modified lines R167-R180
| @@ -158,17 +158,26 @@ | ||
| var isURL = (payload) => payload instanceof URL; | ||
|
|
||
| // ../../node_modules/.pnpm/superjson@2.2.1/node_modules/superjson/dist/pathstringifier.js | ||
| var escapeKey = (key) => key.replace(/\./g, "\\."); | ||
| var escapeKey = (key) => key.replace(/\\/g, "\\\\").replace(/\./g, "\\."); | ||
| var stringifyPath = (path) => path.map(String).map(escapeKey).join("."); | ||
| var parsePath = (string) => { | ||
| const result = []; | ||
| let segment = ""; | ||
| for (let i = 0; i < string.length; i++) { | ||
| let char = string.charAt(i); | ||
| const isEscapedDot = char === "\\" && string.charAt(i + 1) === "."; | ||
| if (isEscapedDot) { | ||
| segment += "."; | ||
| i++; | ||
| const char = string.charAt(i); | ||
| if (char === "\\") { | ||
| const nextChar = string.charAt(i + 1); | ||
| if (nextChar === "\\") { | ||
| segment += "\\"; | ||
| i++; | ||
| continue; | ||
| } | ||
| if (nextChar === ".") { | ||
| segment += "."; | ||
| i++; | ||
| continue; | ||
| } | ||
| segment += "\\"; | ||
| continue; | ||
| } | ||
| const isEndOfSegment = char === "."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review CompleteYour review story is ready! Comment !reviewfast on this PR to re-generate the story. |
Summary
ERR_REQUIRE_ESMerror that occurs when using@trigger.dev/corein CJS environments with Node.js < 22.12Problem
The superjson package v2.x is ESM-only, but Node.js versions < 22.12 don't support
require()of ESM modules. This causes the following error when using@trigger.dev/corein CJS environments:Solution
The fix bundles superjson@2.2.1 using esbuild into both CJS (
superjson.cjs) and ESM (superjson.js) formats in a newvendor/directory. The existing wrapper modules (superjson.tsandsuperjson-cjs.cts) are updated to import from these vendored bundles instead of the npm package directly.This ensures the package works correctly in all environments without relying on Node.js's experimental
require(ESM)feature.Test plan
@trigger.dev/corepasspnpm --filter @trigger.dev/core run buildFixes #2937
🤖 Generated with Claude Code