Skip to content
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

Consider supporting importing files as jsonc / json5 #20374

Open
lucacasonato opened this issue Sep 4, 2023 · 10 comments
Open

Consider supporting importing files as jsonc / json5 #20374

lucacasonato opened this issue Sep 4, 2023 · 10 comments
Labels
cli related to cli/ dir suggestion suggestions for new features (yet to be agreed)

Comments

@lucacasonato
Copy link
Member

lucacasonato commented Sep 4, 2023

import data from "./data.jsonc" with { type: "jsonc" };
import data from "./data.json5" with { type: "json5" };

I'm not to sure about this myself yet, but it may be interesting. What are peoples' thoughts?

@lucacasonato lucacasonato added suggestion suggestions for new features (yet to be agreed) runtime Relates to code in the runtime crate labels Sep 4, 2023
@lino-levan
Copy link
Contributor

Although it may seem like a practical inclusion, I worry that it's straying away from web spec. How would we decide which formats to allow? Would tomls be importable? How about yaml? I don't see a clear way we can draw the line here. I'm open to hearing what you think, I'm just a little skeptical about Deno straying away from spec text.

@Jabolol
Copy link

Jabolol commented Sep 8, 2023

Even though I agree with @lino-levan, since Deno supports deno.jsonc configuration files implementing this would make sense. There are times when one wants to store extra configuration in the deno.jsonc file to reference it in the project programmatically, and this would help.

@lino-levan
Copy link
Contributor

Outside of static analysis, this doesn't provide a lot of benefit imo. Are there enough people asking for jsonc specifically? I'd love to get a sense of if people actually want this.

@scarf005
Copy link
Contributor

i'd like to share my use-use:

  • import fmt property of deno.jsonc
  • feed it as an option to wasm dprint formatter
  • use it for formatting texts programmatically

it's more of an workaround, however once deno exposes fmt as an runtime API (#10731).

@lino-levan
Copy link
Contributor

Perhaps #19322 would be a less hacky way of doing it while not exposing a runtime API?

@pubmikeb
Copy link

pubmikeb commented Oct 8, 2023

JSON is a great (de-)serialization format for apps' configs and data.
However, it doesn't support comments. Luckly, there are two formats, which support comments in JSON: .jsonc and .json5.

As a modern application environment, it would be great to be able to use .jsonc and .json5 instead of classical .json in projects.

Following #20833, please add support importing files as .jsonc.

@lucacasonato lucacasonato changed the title Consider supporting importing files as jsonc Consider supporting importing files as jsonc / json5 Oct 9, 2023
@lucacasonato lucacasonato added cli related to cli/ dir and removed runtime Relates to code in the runtime crate labels Oct 9, 2023
@Malix-Labs
Copy link

Malix-Labs commented Aug 12, 2024

Also note that JSON5 is arguably a better spec than JSONC, see microsoft/vscode#100688

MasterKale added a commit to MasterKale/SimpleWebAuthn that referenced this issue Nov 23, 2024
JSON imports as per https://docs.deno.com/examples/importing-json/ won't recognize .jsonc files. There's an issue at denoland/deno#20374 about supporting importing .jsonc/.json5 files as well but doesn't seem to have much momentum. Feels best not to fight this battle right now.
@SebastienGllmt
Copy link

SebastienGllmt commented Dec 14, 2024

I think { type: "jsonc" } makes sense because deno.json files are actually valid jsonc files (and this isn't planned to change per #20681)

That means that doing import DenoConfig from "../deno.json" with { type: "json" }; actually may fail because the file might be jsonc under the hood. This is counter-intuitive because even the Deno compiler complains, telling you that you can only import json files with type: "json" even though that fails

Note that if you don't want to use fs, you could use type: "text" which is planned (see here and here) (but not implemented yet), but

  1. It's a bit unintuitive to see from "../deno.json" with { type: "text" };
  2. We would have to remove the compiler error telling you that you must use type: "json" when importing .json files because it would actually be the wrong import attribute when dealing with jsonc files like deno.json.

Cascading implications

You could argue the compiler strictness around { type: "json" } would have to be loosened anyway because many other projects allow json5 content in .json files (so you would at least have to support { type: "json" }, { type: "json5" }, { type: "jsonc" } at which point maybe just allowing { type: "text" } doesn't sound so bad

Some people have even suggested allowing to remove the import assertion entirely with a flag like #26157

@scarf005
Copy link
Contributor

How about just allowing importing JSONc as JSON with its comments stripped away?

import data from "./data.jsonc" with { type: "json" }

related: https://douglascrockfordisnotyourdad.technomancy.us

@SebastienGllmt
Copy link

SebastienGllmt commented Dec 14, 2024

How about just allowing importing JSONc as JSON with its comments stripped away?

Be careful because the behavior of { type: "json" } is explicitly mentioned in the import attributes specification because it's also used by web browsers

If you wanted to go this option, Deno would need to generate extra code when transpiling to vanilla JS for browsers when it seems { type: "json" } to make it work, and so it depends if the Deno team team wants to take that approach.

It also doesn't solve the problem that some projects use json5 for their .json files instead of jsonc, so now you'd have to ship a json5 parser in every webpage built with Deno that imports json files

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli related to cli/ dir suggestion suggestions for new features (yet to be agreed)
Projects
None yet
Development

No branches or pull requests

7 participants