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

Support deno.jsonc for Deno config file #211

Closed
binyamin opened this issue Jul 10, 2022 · 4 comments · Fixed by #229
Closed

Support deno.jsonc for Deno config file #211

binyamin opened this issue Jul 10, 2022 · 4 comments · Fixed by #229

Comments

@binyamin
Copy link
Contributor

Summary

Currently, Lume only supports deno.json files. Deno itself supports deno.jsonc files as well. Ideally, Lume would match Deno's behavior.

Brainstorming

  • Deno's standard library includes a JSONC parser in std/encoding. It does not include a method for serializing JSONC (see https://github.com/denoland/deno_std/discussions/1803).
  • We only write to the deno config after the vendor and init commands. That's not too often. It might be worth parsing the JSONC using std/encoding, and doing some RegExp magic to overwrite a portion of the file. Just brainstorming here.

Notes

  • This feature is not in any way critical. I simply wanted to track JSONC support, and to explain why Lume does not in fact support JSONC.
@oscarotero
Copy link
Member

As you said, Lume needs to read and write deno.json files to configure tasks and the import map file. In addition to vendor and init, this file is updated after upgrade and import-map commands.

Personally, I think it was a bad idea from Deno developers to allow to use jsonc here, it's not standard, introduce a unneeded complexity and inconsistency (what happens if deno.json and deno.jsonc exists in the same folder?, why it can be used for deno config but not for import_map.jsonc?), it's hard to parse and, specially to write while keeping the existing comments.

When Deno standard library include a option to parse and stringify JSONC, I'll consider support it, althought I think everybody should use json format.

@kidonng
Copy link
Contributor

kidonng commented Aug 8, 2022

why it can be used for deno config but not for import_map.jsonc?

Because Deno follows the spec, which only uses JSON: denoland/deno#12993

@ooker777
Copy link
Contributor

ooker777 commented Dec 23, 2024

Upgrading Lume still yields this error:

deno task lume upgrade
Task lume echo "import 'lume/cli.ts'" | deno run -A --v8-flags=--max-old-space-size=8192 - "upgrade"
Deno configuration file not found: deno.json

Btw, there is a std lib for jsonc now: @std/jsonc - JSR

@oscarotero
Copy link
Member

@ooker777 You're right. In recent Lume versions the upgrade script has changed and jsonc files are not loaded. I'll fix it.

@oscarotero oscarotero reopened this Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants