vercel / turborepo

Build system optimized for JavaScript and TypeScript, written in Rust
https://turbo.build/repo/docs
MIT License
26.42k stars 1.84k forks source link

[turborepo] .jsonc support (Json with comments) #3793

Open softmarshmallow opened 1 year ago

softmarshmallow commented 1 year ago

What version of Turborepo are you using?

v1.7.2

What package manager are you using / does the bug impact?

Yarn v2/v3 (node_modules linker only)

What operating system are you using?

Mac

Describe the Bug

renaming turbo.json to turbo.jsonc

Expected Behavior

search for turbo.jsonc for fallback

To Reproduce

rename .json to .jsonc

Reproduction Repo

No response

mehulkar commented 1 year ago

@softmarshmallow for what it's worth, you can have comments in turbo.json. I know it doesn't help with things like IDE formatting, but you should at least be able to write comments. Previously discussed here also: https://github.com/vercel/turbo/issues/1470

mehulkar commented 1 year ago

Looks like in VSCode at least you can associate a file with a language, I've done that for this repo as well in #3812. Does that work for you @softmarshmallow?

anthonyshew commented 9 months ago

This is supported behavior! Woohoo!

ari-becker commented 9 months ago

@anthonyshew is it? I tried turbo.jsonc with 1.12.3 and I get an error Could not find turbo.json.

mehulkar commented 9 months ago

turbo.jsonc is not supported, but you can add comments to turbo.json. The only other reason to support is to get native syntax highlighting from your editor, which is possible via user configuration, so it does not seem like a compelling enough reason to add support. @ari-becker can you think of any other motivations to add this?

anthonyshew commented 9 months ago

I suppose the specific implementation described in this issue isn't supported. I was doing some cleanup of old issues and saw the title + not much description so went on the goal, not the implementation. Apologies!

Given that the goal is "add comments to turbo.json", I'd figure we could call the intent of this issue satisfied since turbo,.json supports comments?

@mehulkar Or, alternatively, we could leave it open as a label:good-first-issue?

mehulkar commented 9 months ago

i don’t feel that supporting an alternate file name is valuable, but since the issue was closed for a slightly different reason I wanted to give @ari-becker a chance to reply. I’m in favor of closing unless we discover a compelling reason.

ari-becker commented 9 months ago

@mehulkar it's not even my issue originally, I just watched it for updates since I'm affected by it too 😅

Please note that editors (e.g. WebStorm) can and do identify the comments as an error:

image

This registers as a false-positive in IDEs and negatively impacts attempts to try to keep the number of problems in a project at zero.

Maybe you think that's "not valuable", but IMO not supporting a .jsonc extension is bad developer experience and that's reason enough to keep this issue open - @anthonyshew 's suggestion to keep it as label:good-first-issue sounds good to me.

donhcd commented 9 months ago

agree, my understanding of convention is that .json does not support comments and .jsonc does support comments. It's unexpected (but convenient) that comments work in turbo.json, and editors will work much better if turbo.jsonc is supported. Developers should not need to configure an exception in their editor to turbo.json a JSONC file

mehulkar commented 9 months ago

I agree that native support would be ideal. "valuable" was the wrong choice of words, my apologies. I mean that in my opinion, it doesn't seem worth tradeoff of maintaining that feature. I'll let @anthonyshew make the final call though, as I could be in the minority here.

Webstorm can also configure this file type associations for what it's worth, and I imagine other editors can too.

We configure for VSCode in our repo as well: https://github.com/vercel/turbo/blob/178a2948eeb795ba925838b2380bd5c532a2cd5f/.vscode/settings.json#L12

zirkelc commented 5 months ago

We configure for VSCode in our repo as well:

https://github.com/vercel/turbo/blob/178a2948eeb795ba925838b2380bd5c532a2cd5f/.vscode/settings.json#L12

@mehulkar you comment is so valuable! It should definitely be added to the official docs!

jbreckmckye commented 4 months ago

This approach doesn't work 100% for users of Jetbrains IDEs (Webstorm, IDEA, etc). You can associate turbo.json with JSON5 but not JSONC, but JSON5 includes other features (like non-quoted keys) that turbo.json can't handle.

To use JSON5 as a partial workaround you can right click on the file and Override file type. Alternatively you can set up a general rule in Settings > Editor > File types where you associate turbo.json with JSON5.

skyf0l commented 4 months ago

For now, I'm using a ln -s symbolic link “turbo.json -> turbo.jsonc” because the suggested VSCode config solution isn't great (e.g. prettier doesn't like it + how to make a generic config for a large dev team who each use a different IDE).

I don't think it would be complicated to do what renovate does, which uses a list of possible paths ordered by precedence: image

(https://github.com/renovatebot/renovate/blob/main/lib/config/app-strings.ts)

hiramhuang commented 2 months ago

Workaround for VS Code, add the following config in the workspace/folder config file.

{
  "settings": {
    "files.associations": {
      "turbo.json": "jsonc"
    }
  }
}