search

denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
97.47k stars 5.38k forks source link

Consider deprecating deno.jsonc auto-discovery (and just have deno.json) for a performance reasons #25793

Closed dsherret closed 1 month ago

dsherret commented 1 month ago

Probing for deno.jsonc is quite wasteful and comments are already supported in deno.json. Startup performace could be improved a lot on slow file systems by not bothering to probe for this file.

petamoriken commented 1 month ago

I would rather deprecate .json as I feel uncomfortable using comments with the .json extension.

HasanAlrimawi commented 1 month ago

I'm interested in deprecating deno.jsonc. I believe I'll be ought to remove probing, and as a result I'll have to update all tests and any findings of deno.jsonc to deno.json. As well as there might be some changes in other repos maybe as deno_config. If the scope is wider or there are any other considerations please let me know...

rlidwka commented 1 month ago

Probing for deno.jsonc is quite wasteful and comments are already supported in deno.json.

Are comments supported in every other json file (including package.json and user imports) as well? If not, having files with the same extension but different semantics seems problematic.

On the other hand, jsonc extension is misleading as well, e.g. deno.json supports trailing commas, which not all jsonc implementations do.

I guess deno.json is better, and we'll have to live with the fact that "json for configuration" and "json for data" are two different formats (especially with bun now supporting comments in package.json since April).

petamoriken commented 1 month ago

JSON is the file format defined by RFC 8259 and ECMA 404, and I strongly oppose the use of the .json extension for any other format, even if it's Microsoft's JSONC format.

dsherret commented 1 month ago

It's already the case that deno.json supports JSONC fwiw (similar to tsconfig.json). It's been that way since the start so there would be no change there. The change would be unifying to only deno.json for auto-discovery.

Are comments supported in every other json file (including package.json and user imports) as well?

No, for node compatibility it doesn't support it in package.json and it doesn't support it in import maps.

petamoriken commented 1 month ago

It's already the case that deno.json supports JSONC fwiw (similar to tsconfig.json). It's been that way since the start so there would be no change there. The change would be unifying to only deno.json for auto-discovery.

Then we should deprecate deno.json and move to the deno.jsonc instead of making any more non-standard decisions.

dsherret commented 1 month ago

The deno.json is a local file that isn't distributed (unlike a package.json, which is why comments there are bad). Additionally, a developer can choose to have comments in it or not, so the Deno runtime accepting comments in a file with a json extension is perfectly fine IMO (also see). There's lot of precedence for this and many tools do it for local configuration files.

dsherret commented 1 month ago

I'm going to close this suggestion because based on usages on GitHub this would be too disruptive