Comments (7)
Maybe worth opening another issue for this...
Done: #17
The alternative is to follow the pattern of no-weak-types and by default all suppression errors will error but you can turn them off like [2, { '$FlowExpectedError': false }]. I like this idea more because basically it gives more value to the various suppression error types where right now they all have the same effect.
Sounds good to me. 👍
Additionally to this option object we could add flowTests property...
This might not be necessary once we can do [2, { '$FlowExpectedError': false }]
. 🤔
from eslint-plugin-ft-flow.
So for the first one, you're naming a type variable as $FlowFixMe
? What use case do you have to do that? I believe this rule is made to handle suppression errors (aka flowfixme) in comments.
For your second case, you've made this file strict though you want to have suppression errors which is an anti pattern as per the docs. Any chance you'd consider removing strict mode on these flow tests you run, maybe it doesn't make sense there?
Or maybe the rule just doesn't make sense for your codebase which is fine since it's not part of recommended config 🙂
Tagging original author in case he has any ideas @nvie since tbh I don't use strict mode.
from eslint-plugin-ft-flow.
So for the first one, you're naming a type variable as
$FlowFixMe
? What use case do you have to do that? I believe this rule is made to handle suppression errors (aka flowfixme) in comments.
In this case $FlowFixMe
is a special type and it basically means any
. See: https://flow.org/en/docs/config/options/#toc-suppress-type-string
I didn't realize it's not built-in like other suppressions so I think this should be configurable.
For your second case, you've made this file strict though you want to have suppression errors which is an anti pattern as per the docs. Any chance you'd consider removing strict mode on these flow tests you run, maybe it doesn't make sense there?
I agree with the statement for normal files. But the docs are also saying "just add @flow strict
once all issues have been resolved" which is never the case in flowtests. The whole point of flowtests is to throw errors and mark them as expected via $FlowExpectedError
. That doesn't mean the file cannot follow strict lint rules though. Flowtests are a special case in my opinion.
from eslint-plugin-ft-flow.
oh wow learning something new everyday, suppress_type
is a cool idea!
On the point of this rule it should not handle usage of suppress_type
that goes beyond the specs of the rule and we should be careful it doesn't become a catchall. In fact I would think because it's an alias to any
it can be an option that's added to no-weak-types where it can accept an array of types that your project has configured reflecting .flowconfig
.
For your suppression errors in flow strict files we can add an option to this rule that takes an array of ignored dirs?
Let me know what you think of these ideas.
from eslint-plugin-ft-flow.
I have another improvement to this rule before I address your comments: I think the rule name no-flow-fix-me-in-strict-files
is not great. Something like no-flow-suppressions-in-strict-files
would be more accurate (it's not only about "flow fix me").
I would think because it's an alias to
any
it can be an option that's added tono-weak-types
where it can accept an array of types that your project has configured reflecting .flowconfig.
Here is my point of view: no-weak-types
is a great rule and we are using it to avoid usage of Object
and Function
types. However, we allow any
because sometimes you simply want anything or you want to optimize. No need to go into whether it's good or bad, that's how we do it. For the same reason, I am not planning on disabling suppress_type=$FlowFixMe
because it has its use-cases.
On the other hand, Flow doesn't allow any
in strictly typed files and I want to follow the same logic with types defined in suppress_type
. That's why I think it would be a good addition for this rule (in other words, take into account not only suppress comments but also suppress types). At this moment there is no way how to prevent this.
For your suppression errors in flow strict files we can add an option to this rule that takes an array of ignored dirs?
I thought that ignoring $FlowExpectedError
should be enough because, well, it's "expected". But yeah, it should be fine as long as I can ignore all flowtests (https://github.com/facebook/flow/blob/36eca27dab527f4f300c1a725e39aad5a04dd41e/packages/flow-dev-tools/src/comment/remove-commentsRunner.js#L84-L92) in the whole codebase easily. 🙂
from eslint-plugin-ft-flow.
I have another improvement to this rule before I address your comments: I think the rule name no-flow-fix-me-in-strict-files is not great. Something like no-flow-suppressions-in-strict-files would be more accurate (it's not only about "flow fix me").
yes I had the same thought.. Maybe worth opening another issue for this I'm not sure of the impacts of changing it, it would be a breaking change for anyone using it. Though could be a now or never update since this lib isn't that hugely used yet...
I thought that ignoring $FlowExpectedError should be enough because, well, it's "expected". But yeah, it should be fine as long as I can ignore all flowtests
The alternative is to follow the pattern of no-weak-types
and by default all suppression errors will error but you can turn them off like [2, { '$FlowExpectedError': false }]
. I like this idea more because basically it gives more value to the various suppression error types where right now they all have the same effect.
I'm happy to implement this one if it suits you?
from eslint-plugin-ft-flow.
Additionally to this option object we could add flowTests
property which defaults to being false
which is fair since it's for testing but if a user wants to change it to true
, they can do that also.
Overall the rule structure would be used like
'ft-flow/no-flow-fix-me-in-strict-files': [2, {
$FlowExpectedError: false, // you'd disable this
flowTests: false, // this is the default value but could be changed to true which means to throw error
}]
from eslint-plugin-ft-flow.
Related Issues (19)
- `ft-flow/enforce-suppression-code`: TypeError: Cannot read properties of undefined (reading 'startsWith') HOT 4
- Improve test runner HOT 1
- Consider renaming `no-flow-fix-me-in-strict-files` to something like `no-flow-suppressions-in-strict-files` HOT 1
- improve readme generator
- Function type parameters spacing issues HOT 2
- Recommended configuration not working with enums HOT 7
- Enums support consistent trailing comma
- Enums support unused value
- Switch to use `hermes-eslint` as the recommended parser HOT 14
- Q HOT 1
- Allow mixed type-import-style HOT 1
- Add meta.schema to rules
- Fix mapped types throwing eslint errors
- should depend on hermes-eslint with >=
- New consistent property delimiter rule HOT 1
- Fix all lint errors and enable as part of build
- flow type the repo
- Fix dodgy old file
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from eslint-plugin-ft-flow.