brigand / brigand/babel-plugin-flow-react-proptypes

Forward-uses are treated as uses of external classes

Open
#185 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
427
Forks
42
PR merge metrics
No merged PRs in 30d

Description

In Flow, the following declarations are equivalent:
```js
type Y = number;
type X = {
y: Y;
}
```
and
```js
type X = {
y: Y;
}
type Y = number;
```

This plugin translates the first one correctly, but in the second one
the attribute `y` of the PropTypes generated for `X` refers to `Y` as if
it were an external class, not a Flow type. In practice, this means that
the attribute is equivalent to `y: any`. Presumably, this is because the
conversion scans linearly across the file and has not yet encountered
the definition of `Y`.

This occurs frequently in practice. A structure like
```js
export type CommitData = {
fileToCommits: {[filename: string]: string[]},
commits: {[commitHash: string]: Commit},
};
type Commit = {
author: string,
stats: {[filename: string]: FileStats},
};
type FileStats = {
lines: number,
added: number,
deleted: number,
};
```
is perfectly readable, but is turned by this plugin into
```
var bpfrpt_proptype_CommitData = {
fileToCommits: PropTypes.objectOf(PropTypes.arrayOf(PropTypes.string.isRequired).isRequired).isRequired,
commits: PropTypes.objectOf(function () {
return (typeof Commit === "function" ? PropTypes.instanceOf(Commit).isRequired : PropTypes.any.isRequired).apply(this, arguments);
}).isRequired
};
```
which is effectively useless because `typeof Commit === "undefined"` as
`Commit` does not exist in the code. To get the right structure, one
must permute the source:
```js
type FileStats = {
lines: number,
added: number,
deleted: number,
};
type Commit = {
author: string,
stats: {[filename: string]: FileStats},
};
export type CommitData = {
fileToCommits: {[filename: string]: string[]},
commits: {[commitHash: string]: Commit},
};
```
This is much less readable. The high-level concepts should come first
(why do I care about `FileStats` if I don’t know that I’m talking about
commits or commit data?). But even discarding claims of readability, the
plugin output is clearly doing the wrong thing: equivalent Flow programs
are being translated different, which shouldn’t be the case.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.