Skip to content

fix(runtime): serialize BigInt attribute values instead of throwing - #3471

Open
chatman-media wants to merge 1 commit into
pugjs:masterfrom
chatman-media:fix/bigint-attribute-values
Open

chatman-media wants to merge 1 commit into
pugjs:masterfrom
chatman-media:fix/bigint-attribute-values

Conversation

@chatman-media

Copy link
Copy Markdown

Problem

Fixes #3437.

Passing a BigInt as an attribute value throws instead of rendering it. For example:

pug.render("input(type='hidden' value=whatIsIt)", { whatIsIt: BigInt(10) });

On the current master this throws:

TypeError: Do not know how to serialize a BigInt

(On 3.0.2, as reported in #3437, it produced value=""10"".) The expected output is value="10", exactly as it already works for a Number.

Cause

In pug_attr (packages/pug-runtime/index.js), every non-string value is serialized with JSON.stringify:

if (typeof val !== 'string') {
  val = JSON.stringify(val);
  ...
}

typeof someBigInt === 'bigint', so a BigInt reaches JSON.stringify, which throws for BigInt values. Number works only because JSON.stringify(10) === "10".

This affects both:

  • runtime values (value=local where the local is a BigInt), and
  • compile-time constant attributes, since pug-attrs folds constants through runtime.attr(...) too.

Fix

Coerce bigint to its decimal string before the JSON.stringify branch, mirroring how a Number is rendered and preserving precision beyond Number.MAX_SAFE_INTEGER:

if (typeof val === 'bigint') {
  val = val.toString();
}

Tests

Added BigInt cases to the existing attr test in packages/pug-runtime/test/index.test.js (all escaped/terse combinations, plus a value above Number.MAX_SAFE_INTEGER to confirm precision is preserved). The test fails without the fix (TypeError: Do not know how to serialize a BigInt) and passes with it.

The two failing tests in packages/pug/test/error.reporting.test.js are pre-existing on master (outdated V8 error-message wording, see #3466) and are unrelated to this change.

`pug_attr` serialized any non-string attribute value with
`JSON.stringify`, which throws `TypeError: Do not know how to serialize a
BigInt` for `bigint` values. As a result an attribute whose value is a
BigInt (e.g. `input(value=count)` with `count` a BigInt local) crashed
rendering instead of producing `value="10"` as it does for a Number.

Coerce `bigint` values to their decimal string before the
`JSON.stringify` branch, mirroring how Numbers are handled and preserving
precision beyond `Number.MAX_SAFE_INTEGER`.

Fixes pugjs#3437
@rollingversions

Copy link
Copy Markdown

There is no change log for this pull request yet.

Create a changelog

@seebaermichi

Copy link
Copy Markdown

Thanks for this! I tested it against master (c323ed3e), npm pug@3.0.4 and this branch (e446b7c5).

The fix is right. On master and 3.0.4, a BigInt attribute value throws TypeError: Do not know how to serialize a BigInt from the JSON.stringify in pug_attr. With this PR it renders as its decimal digits, like the equivalent Number (and exactly, beyond Number.MAX_SAFE_INTEGER), and the check sits in the right place: after the toJSON branch (which only handles objects and functions) and before JSON.stringify. Because every attribute path ends in pug_attr, it also fixes !=, doctype html and doctype xml, 0n, &attributes(obj) and mixin attributes. I checked each of those against both the pug-runtime module (what render and compile use) and the copy that compileClient inlines.

  • Suite: 648/650, the same 2 failures as master (V8 error-message wording), snapshots 326/326.
  • The new attr cases fail without the change (TypeError: Do not know how to serialize a BigInt) and pass with it.
  • prettier --check is clean on both files.
  • compileClient output differs from master only in the inlined pug_attr.
  • Nera's test suites and site builds against this branch are green, with byte-identical output.

One note on #3437's output. In a plain Node process, 3.0.2 throws as well. The reporter's value=""10"" is exactly what Pug renders when the app defines BigInt.prototype.toJSON = function () { return this.toString() }. JSON.stringify calls that toJSON on the primitive and quotes the result. With this PR that case renders value="10" too, so it fixes what the reporter actually saw. For apps that carry such a polyfill this is a visible output change, towards what a Number renders.

A small correction to the description. A template cannot give pug-attrs a constant BigInt to fold: value=10n (like p= 10n) is a compile-time syntax error in the lexer, and BigInt(10) is not a constant, so it goes through the runtime path this PR fixes. That doesn't change the fix. BigInt literals in templates are a separate question.

Out of scope, and I wouldn't widen this PR for it: a BigInt nested in an object or array value (data-x={n: n}) still throws from the outer JSON.stringify, as JSON.stringify itself does.

Rolling Versions still says there is no changelog entry. Something like this for pug-runtime would do: "Render BigInt attribute values as decimal strings instead of throwing (#3437)".

Looks good to merge to me.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bigints are parsed incorrectly resulting in escaped double quotes

2 participants