fix(core): add option to use v8 for daemon message serialization to avoid issues when JSON.stringify would fail #30516
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
|
View your CI Pipeline Execution ↗ for commit ff3e197.
☁️ Nx Cloud last updated this comment at |
AgentEnder
force-pushed
the
fix/core/v8-serialize
branch
from
91f6d67 to
4c8bfff
Compare
🐳 We have a release for that!This PR has a release associated with it. You can try it out using this command: npx [email protected] my-workspaceOr just copy this version and use it in your own command: 0.0.0-pr-30516-4c8bfff
To request a new release for this pull request, mention someone from the Nx team or the |
AgentEnder
force-pushed
the
fix/core/v8-serialize
branch
from
4c8bfff to
94d09ba
Compare
🐳 We have a release for that!This PR has a release associated with it. You can try it out using this command: npx [email protected] my-workspaceOr just copy this version and use it in your own command: 0.0.0-pr-30516-94d09ba
To request a new release for this pull request, mention someone from the Nx team or the |
AgentEnder
force-pushed
the
fix/core/v8-serialize
branch
from
9fc6183 to
3f73f1a
Compare
…void issues when JSON.stringify would fail fix(core): v8 serializer should be able to hash tasks feat(core): use v8 serializer for daemon messages by default
AgentEnder
force-pushed
the
fix/core/v8-serialize
branch
from
3f73f1a to
ff3e197
Compare
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| // strings | ||
| (message.startsWith('"') && message.endsWith('"')) || | ||
| // numbers | ||
| /^[0-9]+(.?[0-9]+)?$/.test(message) |
There was a problem hiding this comment.
The regex pattern /^[0-9]+(.?[0-9]+)?$/ contains an issue with the decimal point matching. The .? will match any character followed by an optional quantifier, rather than specifically a decimal point. To correctly match an optional decimal point followed by digits, the pattern should be /^[0-9]+(\\.?[0-9]+)?$/ with the decimal point escaped.
| /^[0-9]+(.?[0-9]+)?$/.test(message) | |
| /^[0-9]+(\\.?[0-9]+)?$/.test(message) |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
| `No pending promise found for transaction "${tx}". This may indicate a bug in the plugin pool. Currently pending promises:` + | ||
| Object.keys(pending).map((t) => ` - ${t}`) | ||
| ); |
There was a problem hiding this comment.
The error message construction will produce unexpected output because Object.keys(pending).map() returns an array that's being concatenated to a string without joining. To properly format the list of pending promises in the error message, use:
`No pending promise found for transaction "${tx}". This may indicate a bug in the plugin pool. Currently pending promises:\n` +
Object.keys(pending).map((t) => ` - ${t}`).join('\n')This ensures each pending promise ID appears on a new line in the error message, making it more readable for debugging.
| `No pending promise found for transaction "${tx}". This may indicate a bug in the plugin pool. Currently pending promises:` + | |
| Object.keys(pending).map((t) => ` - ${t}`) | |
| ); | |
| `No pending promise found for transaction "${tx}". This may indicate a bug in the plugin pool. Currently pending promises:\n` + | |
| Object.keys(pending).map((t) => ` - ${t}`).join('\n') | |
| ); |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
Current Behavior
For really large objects (particularly those containing large strings) JSON.stringify can fail
Expected Behavior
Daemon serialization doesn't fail for the same strings when setting
NX_USE_V8_SERIALIZER=true. This should become the default behavior.Related Issue(s)
Fixes #