[strict-component-boundaries] Exclude node_modues #140
Conversation
| @@ -25,6 +25,14 @@ ruleTester.run('strict-component-boundaries', rule, { | |||
| code: `import {someThing} from '../OtherComponent';`, | |||
| parserOptions, | |||
| }, | |||
| { | |||
| code: `import fs from 'fs';`, | |||
There was a problem hiding this comment.
these 2 added tests are not right, they should have something that would otherwise trigger this rule, but because they are in node modules (or a core package) they would be valid.
| return true; | ||
| } | ||
|
|
||
| if (!resolvedSource) { |
There was a problem hiding this comment.
What do the two checks above do?
There was a problem hiding this comment.
resolvedSource === null if it is a core library
resolvedSource === undefined if it is not installed
finally if we get a string, its the full path to the module.
| return false; | ||
| } | ||
|
|
||
| return resolvedSource.match(/node_modules/); |
There was a problem hiding this comment.
Usually better to use regex.test since it returns a real Boolean
| { | ||
| code: `import eslint from 'eslint';`, | ||
| parserOptions, | ||
| }, |
There was a problem hiding this comment.
Would be good to test that this actually works for a path that is in node modules and that does not adhere to our rules (something/MyComponent)
There was a problem hiding this comment.
see #140 (comment)
There was a problem hiding this comment.
I need to actually install something/MyComponent, otherwise I don't think this will work.
There was a problem hiding this comment.
cartogram
force-pushed
the
fix-only-check-local-modules
branch
from
c6b355d to
17e5718
Compare
|
@lemonmade cleaned things up and added the node_modules test. |
cartogram
force-pushed
the
fix-only-check-local-modules
branch
from
17e5718 to
287915b
Compare
cartogram
force-pushed
the
fix-only-check-local-modules
branch
from
287915b to
f5e3458
Compare
| @@ -32,10 +39,22 @@ module.exports = { | |||
| }, | |||
| }; | |||
|
|
|||
| function isCoreModule(resolvedSource) { | |||
There was a problem hiding this comment.
What is a CoreModule ?
There was a problem hiding this comment.
A module that comes with node core
| { | ||
| code: `import {getDisplayName} from '@shopify/react-utilities/components';`, | ||
| parserOptions, | ||
| filename: fixtureFile('basic-app/app/sections/MySection/MySection.js'), |
There was a problem hiding this comment.
What it does fixtureFile ?
@lemonmade took a stab at this one, let me know what you think of the approach and how I would go about testing this.
The issue I am having is that in order for the
inNodeModulesfunction to work, the package needs to actually be innode_modulesand to test that I need to have a module installed that breaks thestrict-component-importsrule, which I don't.For example, I would need to have
import {Subscription} from 'apollo-client/util/Observable';installed to this project.cc/ @TheMallen