Skip to content
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

import/no-unresolved requires installed node_modules #1627

Closed
sebastian-nowak opened this issue Jan 25, 2020 · 10 comments
Closed

import/no-unresolved requires installed node_modules #1627

sebastian-nowak opened this issue Jan 25, 2020 · 10 comments
Labels

Comments

@sebastian-nowak
Copy link

It seems that import/no-unresolved will complain for valid import statements if node_modules isn't present or it's outdated.

When using a complex build system, npm install is often executed inside a build system sandbox and node_modules might never be present in the original repository where eslint runs (e.g. through IDEs).

Is there a way to prevent import/no-unresolved from complaining about valid import statements for third-party packages that are listed in package.json? I looked at the import/no-extraneous-dependencies rule but it doesn't report any errors if the package cannot be resolved in the first place.

@ljharb
Copy link
Member

ljharb commented Jan 25, 2020

eslint is itself a node_module, one that should only be installed locally (never globally), so node_modules has to be present for eslint to work.

@ljharb
Copy link
Member

ljharb commented Jan 25, 2020

That also means that local devs must always run npm install on their host/local machine in order to get linting.

@sebastian-nowak
Copy link
Author

Hmm, that's not really true. In a large multi-language monorepo it's common to have eslint installed just once (usually in /third_party/bin/eslint or similar) and some build system (such as Bazel or Buck) takes care of running it for various isolated packages. In such scenario, eslint isn't installed globally but it's not in the package's node_modules either. It's invalid to assume that node_modules will be present, it might be managed by a build system.

@ljharb
Copy link
Member

ljharb commented Jan 25, 2020

Not in the js ecosystem it’s not; it’s also a bad practice regardless of how often it’s used.

I don’t think it makes any sense for any js tool to not assume npm install has been ran.

@sebastian-nowak
Copy link
Author

Sure, what you're saying is true for hobby projects and small companies that have the luxury of writing everything in a single language. Your approach certainly works for the majority of projects and it's perfectly fine if you want to support such projects only.

I just want to make sure you're aware that large enterprise software is often stored in a huge polyglot monorepos and it's not just a js ecosystem anymore - it's a huge ecosystem of various languages that have to play nicely with each other. Building such repo is way more complicated than just running npm install possibly followed by npm build and there's no way around it. That's not "bad practice" as you call it, it's necessity. A build system such as Buck has to understand the (multi-language!) dependency graph and take over node_modules to efficiently work with large code bases. Contrary to what you might think, this design works perfectly fine with eslint, has decent support from IDEs such as VSC or IntelliJ and is pretty common. It's just the eslint-plugin-import that's making invalid assumptions. But well, it's your call whether you want to support large codebases or not.

@ljharb
Copy link
Member

ljharb commented Jan 26, 2020

I've used this plugin on large enterprise codebases for many years, so please don't paint this as "hobby vs enterprise". You're describing a very specific kind of enterprise pattern that is not universal, and it's also a very specific mentality that comes from outside an ecosystem and tries to "take over" that ecosystem's idioms to impose a "multi-language" paradigm on it. Any such paradigm that deviates from established idioms is not the best solution to any problem.

It's totally fine if the build process is more complex than npm run build or similar; but npm install isn't optional, and all JS deps should only ever come from an npm install in any modern JS project, at any scale, hobby, professional, or otherwise.

@ljharb
Copy link
Member

ljharb commented Jan 26, 2020

Very specifically, this rule is called "no unresolved". In node, "resolved" has one ONE meaning - require.resolve, which looks in node_modules and thus requires npm install.

If that pattern isn't one you're following, that's fine! However, you'll need to disable this rule.

@austince
Copy link

austince commented May 12, 2020

This issue seems pretty straightforward and dead-in-the-water for this rule, but we (a small company with a polygot Bazel monorepo) are also facing this issue. We're making the choice to move to Bazel for our JS code in step with Angular, which is now built with Bazel.

@sebastian-nowak - do you know of any alternatives to this rule that can provide similar safety in these kinds of repos?

@ljharb would you consider a separate rule for these types of projects still out of the scope of this plugin?

@ljharb
Copy link
Member

ljharb commented May 12, 2020

You’re asking about projects that are out of the scope of this ecosystem, not just this plugin, so i think it’d be best to create your own rule, in your own plugin.

I’d suggest not using an unidiomatic approach to build your JS; whether Bazel is that or not isn’t something I’m familiar enough to judge - but if it doesn’t involve npm install or the equivalent, and if the resolution rules don’t match node’s (or, can’t be provided to this plugin by a custom resolver) then i think you’re setting yourself up for a large amount of unnecessary complexity.

@austince
Copy link

Thanks for your reply - that sounds like a good approach for the rule. I do agree with Sebastian that it's not quite fair to draw the ecosystem line at build systems that don't conform to the most-exact npm approach, as that was once not well-accepted either, but completely see your points.

Yeah, we're not thrilled at the complexity overhead of Bazel, but we're integrating JS within a larger Java repo where we think it's worth it for the improved deployment and repo consolidation.

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

No branches or pull requests

3 participants