-
-
Notifications
You must be signed in to change notification settings - Fork 26.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refuse to build if user is importing something that isn't declared in package.json #1725
Comments
eslint-plugin-node has similar rules for this: |
eslint-plugin-import also has a rule which could help to hint developers about importing transitive dependencies. |
@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. |
@matoilic This one looks right. Want to send a PR adding it? |
One potential issue here is I don't think editing |
@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 |
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. |
That may be the case. I’m not sure.
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. |
Another good point: https://twitter.com/_jayphelps/status/838489432801959936 |
I think this is important enough to not tag as a future idea and instead for 0.10. 😄 |
Sounds good. I’m not sure I’m aware of a good way to make it happen tho. |
Oops 😊 I was thinking of these rules but navigated to the wrong ones in my haste
Measure twice, cut once :D |
I think those two rules are more intended for package creators. When you install |
Pushing back to 0.11. |
Any news ? |
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 |
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. |
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. |
@Timer is this open? |
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
|
Good night. Any updates for this issue? If not, this may resolve the issue. |
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.
The text was updated successfully, but these errors were encountered: