[cli] .ignore file support #4133
Conversation
| ), | ||
| ).toEqual(false); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Love the tests, great work.
cli/src/commands/install.js
Outdated
| .readFileSync(path.join(cwd, libdefDir, '.ignore'), 'utf-8') | ||
| .replace(/"/g, '') | ||
| .split('\n'); | ||
| } catch (e) { |
There was a problem hiding this comment.
Can we scope the catch to only handle the "couldn't find the file" error and otherwise throw? I'm always leery of just silencing all errors. (JS is much worse about being able to do this, which is a bummer...)
cli/src/lib/npm/npmProjectUtils.js
Outdated
| const ignoreDef = cur.trim(); | ||
| if (ignoreDef === '') return acc; | ||
| // if we are looking to ignore a scope dir | ||
| if (ignoreDef.charAt(0) === '@' && ignoreDef.indexOf('/') === -1) { |
There was a problem hiding this comment.
Does this mean @foo/ would not ignore all subfolders? I think this would surprise some people... maybe we could make that more resilient and add a test for it?
cli/src/lib/npm/npmProjectUtils.js
Outdated
| @@ -90,6 +97,18 @@ export function getPackageJsonDependencies( | |||
| if (deps[pkgName]) { | |||
| console.warn(`Found ${pkgName} listed twice in package.json!`); | |||
| } | |||
| const pkgIgnored = ignoreDefs.reduce((acc, cur) => { | |||
There was a problem hiding this comment.
I think you could use .some instead of .reduce and it'll both fail early and read more easily.
|
@AndrewSouthpaw Thanks for suggestions all done! |
|
Any idea when this will be fixed into a new flow-typed version? |
|
I'll publish a release this weekend. |
Closes #4127.
This PR supports a new
.ignorefile that can be added to the rootflow-typeddir of a project where your lib defs currently live. ie:./flow-typed/.ignoreHere you can add lines of exclusions separated by new lines supporting explicit dependencies or scopes
eg the file may look like
Other notes: Tested locally and works pretty well