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

JSONC/JSON5, yaml, toml ... #647

Open
zolomatok opened this issue Aug 23, 2023 · 9 comments
Open

JSONC/JSON5, yaml, toml ... #647

zolomatok opened this issue Aug 23, 2023 · 9 comments
Labels
enhancement New feature or request

Comments

@zolomatok
Copy link
Contributor

zolomatok commented Aug 23, 2023

... you know where this is going 😄

Do you see a future where Civet might support the type-checked imports of "JSON with Comments" files, or maybe even YAML or TOML? Like how TS already supports direct, type-checked JSON imports.

I'm one of those peeps that are heartbroken, heartbroken I tell ya, by the fact that JSON can't have comments. VSCode has its own "JSON with Comments" format to make it not flip out over the comments, but those files fail at runtime in TS when imported. I'd use other data formats, but those can't be imported nicely.

... pwetty pwease?

@STRd6
Copy link
Contributor

STRd6 commented Aug 24, 2023

Are there any links to issues in the TypeScript repo about this? Before implementing such a feature it is important for us to know where TS stands so we can ensure long term compatibility.

@zolomatok
Copy link
Contributor Author

zolomatok commented Aug 25, 2023

Good thinking, but I haven't found any mention of this in the roadmap.

There are two relevant entries in the Issues, this one, which requests support for JSON5 and this one, but that concerns itself with the format of specifically tsconfig.json and not anything else.

Neither have any satisfying resolution. The former is closed with a comment that I think is missing the point, the latter hasn't seem to have received any official response.

Bun purportedly have support for toml imports, but it doesn't really seem to work as stated in the docs. I played around with it and most I could do is import the toml file, but it's unknown. That does work and if you print it, you do see the parsed object tree. But since it's an unknown, you don't have autocomplete and type information, you don't have access to the properties like shown in the docs. At least not a way VSCode won't complain about.

For that to work, you need to create a .d.ts file, according to this discord thread.
But I haven't tried that, cause I don't like polluting my project with .d.ts stuph.

@STRd6
Copy link
Contributor

STRd6 commented Aug 25, 2023

Ok, after reading through the TS issues I think this is a good idea. Here's my thoughts:

  • Allow for parsing different config file extensions based on what modules exist in the local user project
    • If json5 is installed treat .json as JSON5
    • If yaml is installed also look for .yml and .yaml configs
    • If toml is installed look for .toml configs
    • Easy to extend for additional extensions
    • Doesn't bloat the base civet install because these are all optional
    • Update the docs describing that if you want json5 configs all you need to do is npm install --save-dev json5 or yarn add --dev json5, etc.
      • Strongly recommend setting an explicit config path for speed
    • Optimization: If a config path is specified don't load any unnecessary parsers
    • One potential trickyness: if yaml, toml, and json5 are installed from transitive dependencies things could be slower for no obvious reason and people will get "the authentic node.js experience" 😢

I think this will strike a good balance between keeping the civet install small, keeping the config lookup/parsing quick, and allowing an intuitive flexibility of configuration.

@STRd6
Copy link
Contributor

STRd6 commented Aug 25, 2023

For type-checked imports we may be able to auto-generate a .d.ts in the LSP in a similar way.

@edemaine
Copy link
Collaborator

This sounds great! I imagine this would need to be done in the LSP, the CLI, the unplugin, and civetsc (#61) once that exists? (In particular, this can't be done at the Civet transpilation level.) Not exactly easy, but I'd also love to have comments in JSON, and type checking sounds sweet too.

For typing, maybe we could use something like https://github.com/quicktype/quicktype if that's installed locally? Here's a little demo of quicktype from their website:

image

@zolomatok
Copy link
Contributor Author

zolomatok commented Aug 27, 2023

Allow for parsing different config file extensions based on what modules exist in the local user project

Are we talking about tsconfig here (or maybe 🐈.toml :))?
Or just any json/yaml/toml file in the project?

To be honest, I'm not particularly concerned about the format of those particular config files, cause they are never(?) imported in the project and tsconfig at least already has comments. (Although I'm sure it would be nice for the Civet config file to have comments as well.)

What I'm really after is to be able to import any "jsonc"/yaml/toml file in a type-checked way.
But I'm prolly misunderstanding what you mean by config files.

based on what modules exist in the local user project

I'm not sure about this. Seems kinda weird. I don't recall any other package where other installed packages modify the behaviour. I do appreciate the benefits of this, I'm just not a fond of this kind of implicit magic.

Come to think of it, pug has a feature where you can use installed jstransformers, but doing that feels like you're extending the features and this feels like you're modifying base functionality.

It's not like I have a better idea though, so I guess it's fine.

but I'd also love to have comments in JSON, and type checking sounds sweet too.

🫶

@STRd6
Copy link
Contributor

STRd6 commented Aug 27, 2023

  • Allow for parsing different config file extensions based on what modules exist in the local user project

Some of my thoughts on this are combining with #641 so sorry for any confusion. Just trying to think through the general case of "what kind of data/config files can I import in my project" a subset of which is the civet config.

As for the magic lookups I think it would only really apply to the civet config part of this when no explicit config is specified. To opt out of the magic behavior just specify an explicit config.

When importing .json/.yml in your actual .civet files the explicit extension would prevent any mysterious behavior. If there is no yml module installed then we can't really add any type info to imported .yml files and I don't want to include node bindings for every common data file in the core Civet package. If there is a yml module installed odds are it's the same one that the project is using so using it, if available, to generate type info for the LSP sounds 👍

Thanks for opening the issue and I hope this clarifies things!

@STRd6 STRd6 mentioned this issue Aug 28, 2023
@zolomatok
Copy link
Contributor Author

Sure, right, gotcha!
No probs!

@STRd6 STRd6 added the enhancement New feature or request label Sep 10, 2023
@zolomatok
Copy link
Contributor Author

zolomatok commented Nov 5, 2024

For anyone looking for this, if you're using Bun, importing a JSON with comments does work.

  1. If using VSC, have this in your VSC user/workspace settings to make it shush about having comments in a regular JSON file:
"files.associations": {
    "*.json": "jsonc"
},
  1. Just have a plain something.json file (So not jsonc or json5. Apparently jsonc works as well, but VSC couldn't stop complaining about the import. Running the file might have been fine, but I needed VSC to be happy.)
  2. Feel free to have comments in it
  3. Import with import something from './something.json' with { type: 'text' }
  4. You'll have proper type info on something

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants