flow icon indicating copy to clipboard operation
flow copied to clipboard

.flowconfig should use json

Open darrenderidder opened this issue 10 years ago • 34 comments
trafficstars

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

darrenderidder avatar Dec 01 '14 04:12 darrenderidder

+1

ghost avatar Dec 01 '14 20:12 ghost

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.

j201 avatar Dec 03 '14 19:12 j201

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

gabelevi avatar Dec 04 '14 20:12 gabelevi

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.

StyMaar avatar Dec 07 '14 01:12 StyMaar

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).

thomasboyt avatar Jan 24 '15 18:01 thomasboyt

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.

gabelevi avatar Jan 25 '15 20:01 gabelevi

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.

ghost avatar Aug 04 '15 18:08 ghost

+1

monolithed avatar Sep 06 '15 20:09 monolithed

+1 for TOML

j201 avatar Sep 07 '15 19:09 j201

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

jasonkuhrt avatar Oct 09 '15 01:10 jasonkuhrt

+1

m-alikhizar avatar Jun 08 '17 02:06 m-alikhizar

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": {
    ...
  }
}

trevordmiller avatar Jun 16 '17 23:06 trevordmiller

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

TiddoLangerak avatar Jun 19 '17 08:06 TiddoLangerak

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

gabor avatar Jul 20 '17 08:07 gabor

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

kutsan avatar Jul 21 '17 02:07 kutsan

@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)

gabor avatar Jul 21 '17 07:07 gabor

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.

couto avatar Sep 06 '17 08:09 couto

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

dgcoffman avatar Sep 15 '17 21:09 dgcoffman

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

TrySound avatar Sep 15 '17 21:09 TrySound

+1

dineshbyte avatar Feb 19 '18 18:02 dineshbyte

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

RichieAHB avatar Jun 01 '18 09:06 RichieAHB

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

vjpr avatar Jun 01 '18 09:06 vjpr

@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 😛

mrkev avatar Jun 05 '18 03:06 mrkev

You can always use Bucklescript :)

johnhaley81 avatar Jun 05 '18 03:06 johnhaley81

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

mrkev avatar Jun 05 '18 03:06 mrkev

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

  • "flowConfig": {...} in package.json
  • .flowconfig (existing format or TOML?)
  • .flowconfig.json
  • .flowconfig.yml (or .yaml)

PR's accepted?

texastoland avatar Aug 05 '18 09:08 texastoland

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)

dericcain avatar Nov 27 '18 14:11 dericcain

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.

vicapow avatar Feb 01 '19 04:02 vicapow

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.

vjpr avatar Feb 01 '19 21:02 vjpr

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

ghost avatar Feb 03 '19 19:02 ghost