Skip to content

Support allow option in no-unassigned-import, fix #671#737

Merged
ljharb merged 4 commits into
import-js:masterfrom
kevin940726:master
Jun 1, 2017
Merged

Support allow option in no-unassigned-import, fix #671#737
ljharb merged 4 commits into
import-js:masterfrom
kevin940726:master

Conversation

@kevin940726

Copy link
Copy Markdown
Contributor

Add allow option to no-unassigned-import to fix #671.

I basically follow the same logic from no-extraneous-dependencies. With the glob matching start from process.cwd(). I'm not sure that this is the best solution (maybe use regexp like webpack do?), but it works. Please let me know if there is something need to be improved. Big thanks!

  • write tests
  • implement feature/fix bug
  • update docs
  • make a note in change log

@coveralls

coveralls commented Feb 5, 2017

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.03%) to 94.941% when pulling 7a7763c on kevin940726:master into 790dd66 on benmosher:master.

Comment thread docs/rules/no-unassigned-import.md Outdated

This rule supports the following option:

`allow`: An Array of globs. The files that match any of these patterns would be ignored/allowed by the linter. This can be usefull for some build environment (e.g. css-loader in webpack).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/usefull/useful, s/environment/environments

Comment thread src/rules/no-unassigned-import.js Outdated

function testIsAllow(globs, filename, source) {
if (!Array.isArray(globs)) {
return false // default doens't allow any pattern

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/doens't/doesn't, s/pattern/patterns

Comment thread src/rules/no-unassigned-import.js Outdated
'devDependencies': { 'type': ['boolean', 'array'] },
'optionalDependencies': { 'type': ['boolean', 'array'] },
'peerDependencies': { 'type': ['boolean', 'array'] },
'allow': { 'type': 'array' },

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this require that it be an array of strings specifically?

@kevin940726

Copy link
Copy Markdown
Contributor Author

Sorry for the typos, will change it next commit!

Yes, I think it should.

@coveralls

coveralls commented Feb 5, 2017

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.03%) to 94.941% when pulling b060805 on kevin940726:master into 790dd66 on benmosher:master.

@thiagodebastos

Copy link
Copy Markdown

Hey guys! Are we close to this being resolved? It looks like the only PR conflict is the CHANGELOG.md

@ljharb ljharb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM pending a rebase

@ljharb

ljharb commented Apr 10, 2017

Copy link
Copy Markdown
Member

@kevin940726 please rebase on the command line so as to avoid merge commits; if you have trouble, i can do it for you.

@ljharb

ljharb commented Apr 10, 2017

Copy link
Copy Markdown
Member

(also; tests are failing)

@kevin940726

Copy link
Copy Markdown
Contributor Author

My bad, didn't notice that. I'll try my best. 😔

@kevin940726

Copy link
Copy Markdown
Contributor Author

Is the tests failing on master? Or what have I done wrong 😢

@ljharb

ljharb commented Jun 1, 2017

Copy link
Copy Markdown
Member

@kevin940726 sorry for the delay here. I've rebased your branch for you and resolved the conflicts. Tests seem to pass locally, so let's see if this will get them to pass.

@coveralls

coveralls commented Jun 1, 2017

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.03%) to 95.157% when pulling 8f9b403 on kevin940726:master into 28e1623 on benmosher:master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

Add option to no-unassigned-import for allowed glob

4 participants