flow-typed / eslint-plugin-ft-flow

https://www.npmjs.com/package/eslint-plugin-ft-flow
Other
18 stars 11 forks source link

Recommended configuration not working with enums #26

Closed maxcnunes closed 2 years ago

maxcnunes commented 2 years ago

I couldn't figure it out yet, but once I add extends: ['plugin:ft-flow/recommended'] the linter crashes for enum types:

yarn run v1.22.17
$ ./node_modules/.bin/eslint ./app.js

app.js
  22:7  error  Parsing error: Unexpected token, expected "{" (22:7)

✖ 1 problem (1 error, 0 warnings)

Without ft-flow/recommended it works though.

maxcnunes commented 2 years ago

This setting from the recommended configuration that was causing the issue for me:

"babelOptions": {
      "parserOpts": {
        "plugins": [
          "flow",
          "jsx"
        ]
      }
    }

Actually, it is only the "flow" plugin that causes the problem.

I got it solved by setting an empty babelOptions:{} on my eslint configuration while I don't figure out why that setting isn't working for me.

Thanks for the plugin btw 🙇

Brianzchen commented 2 years ago

I'll be honest I've never used flow enums yet, I'll give it a go today and see what could fix it

maxcnunes commented 2 years ago

@Brianzchen It seems this should be configuration instead, I couldn't find a flow plugin for babel at least:

  "parserOptions": {
    "babelOptions": {
      "parserOpts": {
        "presets": ["@babel/preset-flow"],
        "plugins": ["jsx", "babel-plugin-transform-flow-enums"]
      }
    }
  }
Brianzchen commented 2 years ago

Ok I just tested this and this is what I've realised, babelOptions is basically an overwrite to babel.config.js or .babelrc.js specified in your root project, when you don't have parserOptions defined it will just read your babel config instead which should always work, you basically NEED to have that configured otherwise your code doesn't actually run so we're just doubling up code to match eslint with any other babel parsing.

My suggestion here is that we just remove this property completely from the recommended setting, whatever you have defined in your babel config to traverse your code for runtime, testing and now eslint will all be the same. This project won't need to maintain all sorts of settings for different use cases.

Also note that this property was originally added before @babel/eslint-parser was a thing but now pulling from babel configs is the standard.

Is that ok with you @maxcnunes? I can make this change quickly and then ship a patch immediately after

maxcnunes commented 2 years ago

Yeah, makes sense to me, thanks.

Brianzchen commented 2 years ago

Give v2.0.1 a go

maxcnunes commented 2 years ago

That worked, thanks!