facebook / flow

Adds static typing to JavaScript to improve developer productivity and code quality.
https://flow.org/
MIT License
22.09k stars 1.86k forks source link

.flowconfig should use json #153

Open darrenderidder opened 9 years ago

darrenderidder commented 9 years ago

The config file format should be JSON to allow better tooling integration, rather than an ad-hoc format.

ghost commented 9 years ago

+1

j201 commented 9 years ago

JSON is a bit painful for regex because it doesn't support raw strings. I think the current format is reasonable given its similarity to other dotfiles, but it might be worth looking at alternatives like YAML or just providing a tool on npm that can read a .flowconfig file.

gabelevi commented 9 years ago

I'm 100% open to a different format. JSON with only literals would be pretty easy to support. We do have a JS parser, so as long as we don't need to execute code it should be pretty easy.

@j201 is right, specifying regex's would be a little worse in JSON, but it doesn't seem terrible

StyMaar commented 9 years ago

I quite agree with j201 about regex. Dealling with double backslashes is really annoying. Regex are cryptic enough without it ... I would rather have a parser on npm as suggested by j201.

thomasboyt commented 9 years ago

TOML may be a good alternative. It's relatively common (though not as popular as JSON or YAML), very simple, and has parsers for OCaml and JS. It also has a raw string syntax that may make regexes easier (see the bottom of this section).

gabelevi commented 9 years ago

I suppose this would be a good issue to resolve sooner rather than later, as it would be a breaking-ish change (I suppose we could support both for a while).

Do you guys have strong opinions of TOML vs YAML? I do like TOML's simplicity, but there's something to be said for using something that's more mature and well specified. I think I'm leaning towards TOML.

ghost commented 9 years ago

Thank you for reporting this issue and appreciate your patience. We've notified the core team for an update on this issue. We're looking for a response within the next 30 days or the issue may be closed.

monolithed commented 9 years ago

+1

j201 commented 9 years ago

+1 for TOML

jasonkuhrt commented 9 years ago

YAML. Its very common in a range of communities, though in JS JSON generally rules even though it was optimized for machines and YAML for us. YAML is supported by testem, eslint, and surely more.

+1 for YAML.

-- EDIT June 2017: I would no longer advocate for YAML even though I prefer the format. JSON would allow configuration to go into package.json. This is the ideal consolidation I think

m-alikhizar commented 7 years ago

+1

trevordmiller commented 7 years ago

Has there been any progress on this? Would be great if JSON config could be supported as both a standalone file and a property in package.json files as Babel, Jest, ESLint, and many other tools in the ecosystem do so you could do something like:

.flowrc.json or whatever the file is called

{
  "include": [
    "../otherdir/src"
  ],
  "ignore": [
    ".*/build/.*"
  ],
  "libs": [
    "./lib"
  ]
},

Or

package.json

{
  "name": "some-app",
  "version": "1.0.0",
  "scripts": {
    ...
  },
  "flow": {
    "include": [
      "../otherdir/src"
    ],
    "ignore": [
      ".*/build/.*"
    ],
    "libs": [
      "./lib"
    ]
  },
  "babel": {
    ...
  },
  "jest": {
    ...
  },
  "devDependencies": {
    ...
  },
  "dependencies": {
    ...
  }
}
TiddoLangerak commented 7 years ago

I wrote a small utility to transform JSON files to .flowconfig. If you're interested, you can get it here.

gabor commented 7 years ago

please think about comments. currently the flowconfig format supports comments in the file, but JSON does not. this would be a painful regression.

kutsan commented 7 years ago

@gabor Could be like a normal .js file that exports a JavaScript object (module.exports = {/* options here */}) like ESLint and Webpack did.

gabor commented 7 years ago

@kutsan the problem with .js is that Flow is not written in javascript, so running javascript-code might be complicated there (but i don't know the flow-details, so i might be wrong here)

Couto commented 7 years ago

My two cents on this.

https://github.com/davidtheclark/cosmiconfig is becoming quite popular with JavaScript tools.

It allows for JavaScript files (which allow comments and regular expressions), JSON, and YAML files. It also allows a "flowconfig" property in the package.json for those who prefer that approach.

I believe that apart from TOML files (which maybe they can accept a PR) this kinda resolves our format problems and increases the flexibility of storing flow's configuration (file or package.json).

This approach has also been adopted by other tools (Jest/Babel/ESLint/Prettier). I'm not sure if they use cosmiconfig but at least they use the same approach.

dgcoffman commented 7 years ago

I would really like JavaScript config because I hope to someday provide functions to e.g. module.name_mapper.

TrySound commented 7 years ago

@dgcoffman Do you know flow written in ocaml and compiled? Adding js engine will be quite inefficient.

dineshbyte commented 6 years ago

+1

RichieAHB commented 6 years ago

I've closed the issue but I think there is definitely value in having a more JS dev friendly config file ... comments notwithstanding, JSON would be nice for me: https://github.com/facebook/flow/issues/6404

vjpr commented 6 years ago

I vote for .js. Then you can require the config from another location, or dynamically generate it when its used.

mrkev commented 6 years ago

@vjpr I personally am a fan of the .js config in web pack because it's makes it easy to derive development and production configurations from one or the other.

Two issues though:

  1. Flow is written in OCaml, and doesn't really have a JS runtime. It would need to start one (ie, node) every time it want's to read the config.

  2. What config would the config be checked with 😛

johnhaley81 commented 6 years ago

You can always use Bucklescript :)

mrkev commented 6 years ago

@johnhaley81 that will get us OCaml -> JS, not let us run JS in OCaml 😅

texastoland commented 6 years ago

Maybe it's non-trivial but I'd prefer something Cosmiconfig-like that checks

PR's accepted?

dericcain commented 5 years ago

If comments in JSON are a big deal, what about using JSON5? Seems like it has picked up lately with other major tools using it (looking at you VSCode)

vicapow commented 5 years ago

I think this feature was well intended when it was first proposed but by this point, I'm not sure the reformat would be worth the migration effort. That said, if someone has a compelling use case for why JSON would be worth it, feel free to bump this thread or create a new issue.

vjpr commented 5 years ago

The config format is less important than fixing the config options. There are so many hacks needed at present to get Flow working in many cases, which is why .json or .js would be needed. But if these hacks are not needed then the config format doesn't matter so much.

E.g. When you symlink something outside your root, Flow applies filters like ignore from the config on realpaths. So you might need to search your codebase for symlinks and add them to config. But if https://github.com/facebook/flow/issues/7429 is fixed then its less important.

Something that might be useful with json is being able to turn string or regex values into objects which would allow using new regex libs for example.

E.g.

{
  ignores: [{
    regexOcaml: 'blah',
  }, {
    regexGlob: 'blah',
  }]
}

Flow is written in OCaml, and doesn't really have a JS runtime.

I think it would be worthwhile adding one. I'm sure there are a few places where it would be easier to implement something in JS (like the config).

The startup time impact of adding Node would be relatively negligible compared to the typical startup time.

Also, dogfooding Flow in the Flow codebase would be good.

ghost commented 5 years ago

I support you completely the place is beautifully suitable for men and women

jamesisaac commented 5 years ago

@vicapow According to @avikchaudhuri in another thread:

I believe the Flow team is open to more JS-friendly configuration, and we've had some recent internal threads on this topic. I can't give a timeline but I'm optimistic that we will do something to avoid issues like this one.

That was over the subtle differences between OCaml and JS regex (OCaml has no non-greedy capture groups, which must have tripped a few people up).

Might be worth re-opening this if it's part of what's on the team's radar. Maybe Avik could confirm.

EDIT: The docs even currently reference this issue: https://github.com/facebook/flow/blame/master/website/en/docs/config/index.md#L11

vicapow commented 5 years ago

Makes sense. I’ll reopen for now.

0xdevalias commented 4 years ago

For reference/easier findability, these are the flowconfig related issues/suggestions I found that would bring it in line with other tools like eslint, babel, webpack, etc: