gajus / gajus/eslint-plugin-flowtype
[Request] New Rules for Opaque Types
- Dominant language
- JavaScript
- Stars
- 1.1k
- Forks
- 151
- PR merge metrics
- No merged PRs in 30d
Description
### Reference Original Posting: #260
Opaque types provide us with many new ways to use Flow. After implementing them into our project I can say that this feature alone has not only kept us using Flow when we were getting ready to stop, but it has improved things drastically! It's a huge step forward once you start using them properly!
That being said, there are a few new rules that I can see being needed on the linting side. This is copied from the post in [the specific comment](https://github.com/gajus/eslint-plugin-flowtype/issues/260#issuecomment-341355392) per the request of @valscion
Probably a good time for any other ideas on what may be appropriate from others as well.
---
I successfully integrated opaque types into my project and first of all -- game changer -- absolutely amazing to have.
I have an example of them all in use that could be used as a basis in our [flow-type-transformer](https://github.com/Dash-OS/flow-type-transformer) package here:
https://github.com/Dash-OS/flow-type-transformer/tree/master/tests/flow/opaque-transformation
these do not have any errors for me currently with eslint
So defining an opaque type as previously shown will give us the error:
```js
opaque type MyType = {
one: 'two',
};
```

The more complex problem will come in when trying to actually determine if an opaque type is "useless". In order for an opaque type to ever really match it needs to be returned by a function within the same file that it is defined or exported as a value directly.
At the moment this actually will pass eslint tests:
```js
export opaque type Dash$ResponseWrapper =
| MyType
| Dash$HandshakeResponse
| Dash$SuccessWrapper
| Dash$ErrorWrapper;
opaque type MyType = {
one: 'two',
};
```

But the `MyType` is actually still "useless" because it can never really match anything since I don't provide it to the user through any specific means.
The follow methods should make an opaque type legal (among others I am sure, identifying the various ways an opaque type can be provided to the user will be the difficult part I think):
```js
// now b is an opaque type and that value is the only way to use the opaque type
export const b: MyType = {
one: 'two',
};
```
```js
// by exporting the type through a function call
export function getMyType(): MyType {
return ({
one: 'two',
}: MyType);
}
```
What I am not sure about currently, but I suspect it is the case, is if something like this allows opaque types to be used. If so, then it is going to be significantly harder to track and handle whether an opaque type is actually "useless".
```js
const Value: Map<*, *> = new Map();
export function one() {
Value.set('mytype', ({ one: 'two' }: MyType));
}
export function two() {
return Value;
}
```
---
It stands to reason that a few rules could probably be looked into:
- **no-unused-opaque** - Simply checks if an opaque is defined but never used anywhere. This is pretty much how it works currently but it reports `no-undef` instead.
- **no-useless-opaque** - Checks if an opaque type is created and either never used or exported. If exported, there must be some way of getting the opaque type.
- **no-local-opaque** - Requires that the opaque type be exported. This won't always be the desired effect since there may be times the opaque type will be checked internally but provided in some other way (the user wont be able to check it but internally we still can).
- **require-opaque-getter** - For example, if an opaque type is provided, then it would be expected that at least one exported function will directly return that type as a value. Perhaps as a union returning multiple opaque types or directly or some other means.
- **no-opaque-transform** - While this is a feature in my mind, it could be desirable to stop a person from killing the opaqueness of a type. This can be done, for example, by doing this `$Shape<{ ...MyOpaqueType }>` -- the response of which is no longer opaque. Not 100% sure on the specifics of this at this time -- I use this pattern currently though and definitely would not personally use a rule like this.
- **opaque-defined-first** - This or similar to enforce all opaque types being defined at the top of the file before any standard type definitions. This could work similar to the react lifecycle methods sort allowing to define if opaque should be first, last etc.
```js
export opaque type MyType = {
one: 'two',
};
export function getMyType(): MyType {
return ({
one: 'two',
}: MyType);
}
```
Anyway - seems like it could get pretty specific here. Just some ideas. I am seeing the power of opaque types now though and its definitely very exciting. It's already caught 3-4 bugs that have been in our backend API for 3+ years without us knowing.
Contributor guide
Research direction
Start with the referenced discussion in issue #260 and the flow-type-transformer tests under tests/flow/opaque-transformation. The request lists several possible rules but does not select one or define complete behavior, so first narrow the scope and acceptance criteria before locating the relevant ESLint rule code. Done should mean one agreed opaque-type rule is implemented with its behavior and edge cases covered.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript
- Domain
- tooling
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100