fix(language): keep official schema serialization round-trippable#201
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8693bd8ca3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
8693bd8 to
e516653
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e516653838
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| valueConverter: ValueConverter, | ||
| ): JsonValue { | ||
| return this.isPlainScalar(node) | ||
| ? (valueConverter(node.getValue()) ?? nodeConverter(node)) |
There was a problem hiding this comment.
When a numeric schema keyword such as minimum is backed by a plain BigIntegerNumber outside JavaScript's safe range, NodeToMapListOrValue.handleValue converts it to a string, and this branch emits that string as the bare schema value. NodeDeserializer.validateSchemaKeywordShape only accepts a number or an explicit numeric scalar node for these keywords, so blue.nodeToJson() can now produce schema.minimum: "9007199254740992" that jsonValueToNode() rejects; fall back to the explicit node form whenever the converted scalar is not numeric.
Useful? React with 👍 / 👎.
| valueConverter: ValueConverter, | ||
| ): JsonValue { | ||
| return this.isPlainScalar(node) | ||
| ? (valueConverter(node.getValue()) ?? nodeConverter(node)) |
There was a problem hiding this comment.
For schema.enum values that are plain large integers, this flattening path converts a BigIntegerNumber outside the safe JSON range into a string and emits it as a bare enum item. On readback that item is parsed as Text rather than Integer, whereas the previous per-node serialization would include the inferred Integer type, so round-tripping a numeric enum such as 9007199254740992 silently changes the allowed value's type.
Useful? React with 👍 / 👎.
| const value = node.getValue(); | ||
| const type = node.getType(); | ||
| if (isBigNumber(value) || typeof value === 'number') { | ||
| return type === undefined || NodeDeserializer.isNumericType(type); |
There was a problem hiding this comment.
Explicit numeric schema constraints authored with the documented shorthand type: Integer and a quoted value are rejected here because getValue() only coerces strings when the type node carries the core type BlueId; for an inline alias it returns the raw string, so the numeric check fails before isNumericType(type) can recognize the alias. This makes valid inputs like schema.minimum: { type: Integer, value: "9007199254740992" } fail deserialization even though the validator is intended to allow explicit numeric scalar nodes.
Useful? React with 👍 / 👎.
1 participant
No description provided.