oven-sh / bun

Incredibly fast JavaScript runtime, bundler, test runner, and package manager – all in one
https://bun.sh
Other
73.29k stars 2.69k forks source link

Implement JSON5 support as a module #3175

Open Kruithne opened 1 year ago

Kruithne commented 1 year ago

What is the problem this feature would solve?

JSON is one of, if not the, most widely used format for data-interchange in the JavaScript ecosystem, but it lacks a number of quality-of-life features that I've seen people requesting from runtimes.

What is the feature you are proposing to solve the problem?

Instead of supporting additional features in some custom superset in the standardized functions (JSON.parse, JSON.stringify, import, etc) which could potentially break existing functionality, cause confusion or just be unwanted by developers, I propose that JSON5 support is implemented as it's own module in Bun.

Reference: https://json5.org/

example.json

{
    "foo": "bar" // This controls a thing.
}

example.ts

const text = await Bun.file('test.json').text();
console.log(JSON.parse(text));

// 1 | 
// 2 | JSON.parse(text);
//   ^
//SyntaxError: JSON Parse error: Unrecognized token '/'
//      at /example.ts:2:0

import { parse } from 'bun:json5';
console.log(parse(text));

//{
//  foo: "bar"
//}

Some of the key improvements JSON5 offers over standard JSON:

Why should Bun implement this? Bun is currently setting a trail of doing things differently and implementing things that developers actually want/use rather than just following the status quo. Implementing support for JSON5 as it's own module doesn't interfere with any existing code and allows people who want to stick to the standardized JSON to continue their lives unchanged.

JSON5 has been adopted by large projects, including Chromium.

What alternatives have you considered?

Some discussion on the Bun Discord from 2022 brought up the mention of extending the base JSON implementation to support things such as comments. This provides a level of confusion to users who are expecting JSON.parse() / JSON.stringify() to behave the same as they do in all other contexts, and now means that Bun has a superset of the official JSON standard that it needs to define and make known.

JSON5 is a defined specification that can be directly pointed at for the functionality of the module, and developers are explicitly aware that they are opting in to the extended functionality.

How Bun.file.json() will handle this is something I haven't considered, since Bun.file is obviously bun-specific, so it comes down to the owners to decide if the Bun-specific .json() should follow the standard, use the module implementation, or have both available.

As a closing note, I'll leave this extract from the JSON5 website as additional motivation for supporting it in Bun.

JSON5 was started in 2012, and as of 2022, now gets >65M downloads/week, ranks in the top 0.1% of the most depended-upon packages on npm, and has been adopted by major projects like Chromium, Next.js, Babel, Retool, WebStorm, and more. It's also natively supported on Apple platforms like MacOS and iOS.

Jarred-Sumner commented 1 year ago

This would be pretty straightforward for us to implement.

tsconfig.json is comments + trailing commas, which bun already supports . It's likely a few more flags to pass into js_lexer.zig (and would need to be a separate instance) followed by an enum value in options.Loader, adding .json5 to the list of default extensions and then one more switch statement case in the bundler for getting the ast. I would guess a PR for this is around 300 LOC (excluding tests). The main changes are adding more flags to js_lexer.zig which would include conditionally enabling the JS string & number handling (assuming that JSON5 behaves very similarly to JS for these things, but I haven't read the spec)

felipenmoura commented 1 year ago

Awesome idea. This is one of those things that would make some eyes shine with tears XD Specially for package.json, imagine being able to use package.json5 🤩

cwtuan commented 1 year ago

Currently, NPM does not offer JSON5 support, which can be limiting for developers who rely on the enhanced functionality provided by JSON5.

JSON5 is a popular extension to the standard JSON format, offering features like comments, trailing commas, and more expressive data types. I believe that enabling JSON5 support in bunjs would greatly enhance the development experience for users who prefer JSON5 over standard JSON.

Usage Trends of JSON5 image

christophsturm commented 9 months ago

pnpm supports json5, but node does not support it, so i can not use it for example in a sveltekit project that uses node. https://github.com/sveltejs/kit/issues/8918 json5 support in bun would definately be a reason for me to use bun.

rlidwka commented 9 months ago

Currently, NPM does not offer JSON5 support, which can be limiting for developers who rely on the enhanced functionality provided by JSON5.

Don't use npm then? Fortunately, there are alternatives around.

pnpm does support package.json5 as well as package.yaml. I switched to it recently because of it's installation speed (was using yarn earlier).

I'm reviewing my tools right now for the possibility of using package metadata in json5/yaml (because commenting out reasons for each locked dependency is quite important), that's how I've just found this issue.

cwtuan commented 9 months ago

Currently, NPM does not offer JSON5 support, which can be limiting for developers who rely on the enhanced functionality provided by JSON5.

Don't use npm then? Fortunately, there are alternatives around.

pnpm does support package.json5 as well as package.yaml. I switched to it recently because of it's installation speed (was using yarn earlier).

Thanks for the information. It seems that pnpm is gaining more popularity than yarn and npm. I'll give it a try in my projects.

TheDirigible commented 7 months ago

Another reason would be, import of a JSON file fails with a useless error message if the file isn't strict JSON.