eas-cli icon indicating copy to clipboard operation
eas-cli copied to clipboard

eas.config.js or comment schema

Open ericvicenti opened this issue 1 year ago • 6 comments

Do you understand that any feature requests opened in this repository will be closed?

Yes

ericvicenti avatar Aug 21 '22 12:08 ericvicenti

feel free to close this as its low-pri

my eas.json has no place to put comments or descriptions. Here's the only sad way I know:

{
  "build": {
    "development": {
      "env": {
        "DESCRIPTION": "Used for development of the app on simulators."

this could be fixed by adding support for eas.config.js cause JS is kind enough to support comments

alternatively we could get a .description field in the build config? or "//": "whatvs"

ericvicenti avatar Aug 21 '22 12:08 ericvicenti

If we used https://json5.org/ for eas.json, would it solve the comments problem for you?

dsokal avatar Aug 22 '22 14:08 dsokal

i think we may want to avoid json5, but i don't feel too strongly and i'm open to being convinced. youtuber voice: let me explain.

we used to support json5 in app.json but we decided it wasn't worth the extra complexity and the dependency. one thing we did wrong there was we supported json5 in files with the .json extension, which didn't work great with editors like vscode - you had to manually choose json5 as the filetype, iirc. overall, it just seemed to confuse people.

  • if we did support json5 here, we would need to do it in eas.json5
  • we'd have to decide: which takes priority if both exist? eas.json or eas.json5?
  • we often write to eas.json, so we'd need to be able to write to eas.json5 while preserving comments (this is possible, but i believe it would require installing a package like golden-fleece or jju)

i do not want to support eas.config.js because i don't see a good case for it yet except comments and it is quite complicated to do this well.

brentvatne avatar Aug 23 '22 00:08 brentvatne

@brentvatne

one thing we did wrong there was we supported json5 in files with the .json extension, which didn't work great with editors like vscode

That's interesting. I tried that on my own and you're right. I always thought it just works (with .json extension). What's even worse is that it seems you need a plugin for highlighting. It also doesn't work out of the box in Sublime.

overall, it just seemed to confuse people.

Using .json5 shouldn't be the default. If someone decides they need the features it brings, like comments, then it shouldn't be confusing that there are extra requirements.

if we did support json5 here, we would need to do it in eas.json5

I think it makes sense.

we'd have to decide: which takes priority if both exist? eas.json or eas.json5?

In my opinion, it should print a warning and use eas.json.

we often write to eas.json, so we'd need to be able to write to eas.json5 while preserving comments (this is possible, but i believe it would require installing a package like golden-fleece or jju)

We don't write to eas.json as often as you could think. There are only two places that write to the file:

  • https://github.com/expo/eas-cli/blob/main/packages/eas-cli/src/build/configure.ts#L103 - creating eas.json if it does not exist - we don't need to handle the eas.json5 case here
  • https://github.com/expo/eas-cli/blob/main/packages/eas-cli/src/project/remoteVersionSource.ts#L36 - when setting up remote version management As you said, this is not a blocker and there are libraries that preserve comments, etc.

In my opinion, the request sounds reasonable and we could add support for .json5 files.

dsokal avatar Aug 23 '22 09:08 dsokal

~ok let's do it with the json5 extension then~ actually i'm still not totally sure about this, because then how do we explain not supporting eas metadata as json5 and app.json as json5? if we do it for one config file, any of our json config files should support it. i'm not sure it's worth that trouble

brentvatne avatar Aug 23 '22 19:08 brentvatne

another followup - we actually still support json5 in app.json still, just don't document it. when we write to app.json and it's json5, the formatting is lost.

@EvanBacon also pointed out that tsconfig.json supports json5. maybe i'm coming around to just quietly supporting it?

brentvatne avatar Aug 23 '22 22:08 brentvatne