Skip to content

Conversation

@sapphi-red
Copy link
Member

@sapphi-red sapphi-red commented Oct 8, 2024

Description

This PR

  • adds support for namedExports with stringify: true
  • adds support for stringify: 'auto'
  • changes the default value of stringify to 'auto' from false

.

It is known that embedding an JSON serializable object as a string reduces the parse cost than embedding the object directly for large objects. This was possible by setting stringify: true, but that made all JSON imports into serialized format. Also stringify: true did not support namedExports, so it was not possible to enable it by default.

@sapphi-red sapphi-red added p2-nice-to-have Not breaking anything but nice to have (priority) breaking change labels Oct 8, 2024
@bolt-new-by-stackblitz
Copy link

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

Comment on lines 53 to 55
// use 10kB as a threshold
// https://v8.dev/blog/cost-of-javascript-2019#:~:text=A%20good%20rule%20of%20thumb%20is%20to%20apply%20this%20technique%20for%20objects%20of%2010%20kB%20or%20larger
(options.stringify === 'auto' && json.length >= 10 * 1000)
Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe we should benchmark this to find out whether the threshold has changed over the last five years.

Copy link
Member Author

Choose a reason for hiding this comment

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

I ran the benchmark: https://github.com/sapphi-red/json-parse-benchmark

It's difficult to measure where the threshold is due to the overhead of starting the JS engines. But the perf benefit still exists. Probably fine to go with 10kB for now.

@sapphi-red sapphi-red added this to the 6.0 milestone Oct 16, 2024
Copy link
Member

@ArnaudBarre ArnaudBarre left a comment

Choose a reason for hiding this comment

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

Didn't read all the code but the API looks good to me!

@bluwy bluwy merged commit b80daa7 into vitejs:main Oct 23, 2024
13 checks passed
@sapphi-red sapphi-red deleted the feat/json-stringify-auto branch October 24, 2024 00:40
moonlitusun pushed a commit to moonlitusun/vite that referenced this pull request May 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change p2-nice-to-have Not breaking anything but nice to have (priority)

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants