Skip to content

Handle NaN and Infinity in JSON stringify function #108837

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aaronfranke
Copy link
Member

@aaronfranke aaronfranke commented Jul 21, 2025

The JSON specification does not support NaN or Infinity values. However, it does support scientific notation. https://www.json.org/json-en.html

Without this PR, trying to export JSON containing NaN or Infinity will result in invalid JSON, which cannot be parsed back by Godot, nor can it be parsed by web browsers.

With this PR, trying to export JSON containing NaN and Infinity will result in valid JSON, with Infinity being replaced with extremely large numbers, and NaN being replaced with null. These large values are read in as Infinity both by Godot and by web browsers, and this seems to be a common trick in the JSON ecosystem. See graphite-project/graphite-web#813

The unit tests added in this PR include examples of round-trip parsing, and the same tests fail without the other changes.

@aaronfranke aaronfranke added this to the 4.6 milestone Jul 21, 2025
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good to me. This makes sense as a best-effort attempt to serialize JSON while being as close as possible to the input.

@aaronfranke aaronfranke force-pushed the json-handle-nan-inf branch from 5211b03 to cc25266 Compare July 22, 2025 16:38
@Joy-less
Copy link
Contributor

Why not serialize NaN and Infinity as "NaN" and "Infinity" (or "-Infinity")? This is what C# does on an opt-in basis (link).

@aaronfranke aaronfranke marked this pull request as ready for review July 26, 2025 01:05
@aaronfranke aaronfranke requested review from a team as code owners July 26, 2025 01:05
Copy link
Member

@Ivorforce Ivorforce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are apparently quite a few JSON parsers that use strings or 'keywords' to encode NaN or infinity even though it's not JSON compliant.
Python has a parameter for this (allow_nan). I think we should copy this idea, as I can imagine some people want this technically uncompliant JSON behavior.

@aaronfranke
Copy link
Member Author

aaronfranke commented Jul 27, 2025

Two problems with the string representation:

  • If a field is specified as a number in a JSON schema, "Infinity" is not of that type, so such output would not be valid for that schema, while 1e99999 is still compliant.
  • If reading in data without any schema or other type known in advance, it will be interpreted as a string instead of a number, which means that the exporter would change the data type, while 1e99999 is always imported as a number.

For NaN, null also isn't a number, so either way breaks both of these cases, but it at least implies that the data is "invalid" while still being readable as JSON, unlike "NaN" which looks like a valid string.

Anyway, if we want this functionality, it must be opt-in (as already proposed above, so, good). However, this would mean we need to add a parameter to stringify, which would change the API, and we'd need to register a compatibility method. This could be done as a separate PR, or if it's really strongly wanted I guess I could put it in this PR. But I'm thinking that this would need a proposal, while this PR by itself is good as-is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants