Refuse to build if user is importing something that isn't declared in package.json

[gaearon]

I think Brunch does this, if I recall correctly.

This is an important feature because people often misunderstand that transitive dependencies aren’t supposed to be importable.

See #1714 and #1622 for examples.

[jokeyrhyme]

eslint-plugin-node has similar rules for this:

• https://github.com/mysticatea/eslint-plugin-node/blob/master/docs/rules/no-missing-require.md

• https://github.com/mysticatea/eslint-plugin-node/blob/master/docs/rules/no-missing-import.md

[matoilic]

eslint-plugin-import also has a rule which could help to hint developers about importing transitive dependencies.

• https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/no-extraneous-dependencies.md

[gaearon]

@jokeyrhyme I don't think these are similar. The rules you link to protect against importing a non-existing file (which would break the build anyway). But I'm talking about a file that exists but comes from a transitive dependency.

[gaearon]

@matoilic This one looks right. Want to send a PR adding it?

[gaearon]

One potential issue here is I don't think editing package.json would trigger an eslint-loader recheck, so the user would be stuck with a misleading message in our setup.

[matoilic]

@gaearon Sure, I can work out a PR. What do you think about importing dev dependencies? Should there be a warning / error when importing a dev dependency, especially in a production build?

I'm not sure on how to trigger a recheck or rebuild when editing package.json. Could a Webpack plugin be a solution? I haven't come across one which does something like that. We might need to create one to solve the issue.

[mnquintana]

What would you think about implementing this upstream in webpack? This seems like it'd be super helpful for all / most projects, not only React projects / projects using that ESLint plugin.

[gaearon]

Could a Webpack plugin be a solution?

That may be the case. I’m not sure.

What would you think about implementing this upstream in webpack?

It may be better longer term but if we can prototype this feature here and figure out the edge cases sooner, that might work out just as well. IMO plugins like this end up good when they start in userland, and if proven useful, move into the core.

[gaearon]

Another good point: https://twitter.com/_jayphelps/status/838489432801959936

[Timer]

I think this is important enough to not tag as a future idea and instead for 0.10. 😄

[gaearon]

Sounds good. I’m not sure I’m aware of a good way to make it happen tho.

[jokeyrhyme]

Oops 😊

I was thinking of these rules but navigated to the wrong ones in my haste

• https://github.com/mysticatea/eslint-plugin-node/blob/master/docs/rules/no-unpublished-require.md

• https://github.com/mysticatea/eslint-plugin-node/blob/master/docs/rules/no-unpublished-import.md

Measure twice, cut once :D

[matoilic]

I think those two rules are more intended for package creators. When you install react-scripts the NPM registry will only contain "published" files and thus only files which already pass the check. That's how I understand it. But those rules might be useful to solve #1720.

[gaearon]

Pushing back to 0.11.

[Ayc0]

Any news ?
@matoilic

[mattfysh]

This would be a great addition for junior engineers who don't understand the reason their import works is due to hoisting and/or node_modules flattening

In the meantime, I'm setting up depcheck

[stale]

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

[stale]

This issue has been automatically closed because it has not had any recent activity. If you have a question or comment, please open a new issue.

[mdaj06]

@Timer is this open?

[nikikalwar]

Hi @Timer,

The behaviour is expected because you are trying to import something which is not there.

Let's say If I am importing with the below code:

import {debounceTime} from 'rxjs';

so, for me to access the debounceTime Property from rxjs, i need to have the rxjs object right? so as you said earlier that the build is getting refused because user is importing that isn't declared in the package.json and whether it is declared it doesn't matter.

I imported rxjs which was not declared and build failed so, I installed the package, deleted it's entry from package.json and ran the app with no errors along with the import statement.

The thing is we are importing from node_modules not from package.json so, in the build.js file we can add a warning message saying , the $package is not installed and suggest the user to install the package instead of just saying the below error:

Failed to compile.

Module not found: Error: Can't resolve 'rxjs' in 'F:\github\test3\my-app\src'

Why? Because with the module not found is kind of too vague as a user warning

else { console.log(chalk.red('Failed to compile.\n')); printBuildError(err); console.warn(" Please install the unresolved dependency and then run npm build") process.exit(1);

[BraianS]

Good night.

Any updates for this issue? If not, this may resolve the issue.
Thanks.