Possible readme error for eslint-import-resolver-node

[msikma]

Hi there. I'm currently attempting to get this plugin working with (the equivalent of) a custom NODE_PATH. Was looking at the eslint-import-resolver-node documentation, which says to add an import/resolver key to the .eslintrc file, but after checking the code I see that it actually looks for import/resolve (in lib/core/resolve.js).

I'm not sure if I'm missing something, or if there's a discrepancy.

Relevant versions:

• [email protected]
• [email protected]
• [email protected]

[msikma]

Just to follow up, I was able to get it to work this way, using this .eslintrc with the following options:

"settings": {
  "import/resolve": {
    "paths": ["src/"]
  }
},

It seems that the contents of import/resolve is passed on directly to resolve, so it's not wrapped inside a node key like in the readme.

[benmosher]

Yeah, that is leftover from v0.10. I left the default behavior to minimize breakage to any config in the field.

Going forward, the same config can b provided as indicated in the resolver-node README--though I'll double-check it for accuracy. Whipped it together trying to get the Webpack resolver out this week.

On Nov 29, 2015, at 08:35, Michiel Sikma [email protected] wrote:

Just to follow up, I was able to get it to work this way, using this .eslintrc with the following options:

"settings": {
"import/resolve": {
"paths": ["src/"]
}
},
It seems that the contents of import/resolve is passed on directly to resolve, so it's not wrapped inside a node key like in the readme.

—
Reply to this email directly or view it on GitHub.

[benmosher]

Ah, sorry, the version number info didn't make it in the email. That is indeed the only way to configure paths before v0.11--I just shipped the resolvers with 0.11.

If you want to see the documentation for v0.8.1, you should be able to switch to the tag for that version. I believe that version of the README documents import/resolve, which does indeed pass straight through to the resolve.sync options.

[benmosher]

@msikma: I was just reading through your ESLint Gitter messages, and got a better understanding of what you were after.

Would it be simpler to have the Node resolver use NODE_PATH, if defined on process.env? Would be fairly straightforward.

I generally bake my NODE_PATH into build scripts as an export NODE_PATH=./whatever up top. Seems like it would follow that the resolver would pick this up and run with it, vs. having to configure it in again in .eslintrc.

[msikma]

Would it be simpler to have the Node resolver use NODE_PATH, if defined on process.env? Would be fairly straightforward.

Oh yeah, I think that would make sense, definitely. To my knowledge the only way to have your own import directory is to specify it using NODE_PATH (unless you're using Webpack or something else). There are some other hacks (such as a custom require() hook) but this is the proper way to do it I believe. So I think it'd be good to use it for the resolver as well. 👍

[alondahari]

Hi,
Not sure where you stand with this, but it's not working for me (getting a false negative on files relative to my NODE_PATH declared in the scripts).
I tried defining paths as described above, no luck either.
Had to disable the rule for now.
Am I doing something wrong?

[ShaunEgan]

I am having the same problem as @jazzdragon. Have tried both methods and still getting these missing require hits.

[ljharb]

NODE_PATH is deprecated and considered a bad practice; it should work while node supports it, but I'd recommend migrating away from it.

[alondahari]

What's a good alternative?

[ljharb]

All your requireable things should be in the local node_modules directory per the require.resolve algorithm, which is where dependencies go. There is no better alternative.

[ShaunEgan]

So I know this is probably getting off track of this thread, but regarding the deprecation. I did a bunch of reading and the recommended practices. They all seem to recommend keeping tests side by side with the files they test for.

So we usually keep our tests separate. We use the NODE_PATH to have the tests have the same require root path as our source. Without this, am I right in assuming that tests would have to have ../../ chains to make them work, or is there a better alternative.

The other approach I saw was that you can reference packages in package.json using the file: prefix. Using this, would it mean that every service with an application should be written this way? I don't have a problem with this, I just haven't seen it replicated anywhere.

Super interested in recommendations :)

[ljharb]

@ShaunEgan i highly disagree that colocating tests is a best practice; I think the opposite. However, when keeping tests separate, I simply use ../../ in my test files, yes - which is correct, because that's how node's require algorithm works.

[ShaunEgan]

Cool, I agree with your approach. I really dislike them all in the same place. Thanks for the advice!

[alondahari]

To further go off topic here, I'm really not sure why NODE_PATH is considered a bad practice, especially if it works great and makes code more readable and easier to maintain.
It makes it so I can move one file without breaking everything else.
To me, this:

import MyComponent from 'components/MtComponent'

makes much more sense than:

import MyComponent from '../../../../components/MyComponent

[mockdeep]

We're setting a local path to our project js files. Relative paths are just too error prone and frustrating ("ugh, forgot/added an extra ../ again!"). We put it one directory up so that we can prefix and make it more clear what is coming from our project vs what comes from external node_modules, so import * from 'js/responses/action_creators' vs import React from 'react'.

NODE_PATH is not actually deprecated and it's still officially supported in their docs, as of version 7.8.0, though admittedly they do discourage it. Many tools these days take some sort of configuration that allow you to not mess with NODE_PATH, however:

Jest takes the configuration modulePaths:

{
  "modulePaths": [
    "<rootDir>/app/assets/",
    "<rootDir>/vendor/assets/javascripts/",
  ]
}

Webpack takes the configuration resolve.modules:

config = {
  resolve: {
    extensions: ['.js'],
    modules: [
      path.resolve('app/assets'),
      path.resolve('vendor/assets/javascripts'),
      path.resolve('node_modules'),
    ],
  }
}

Mocha doesn't have a way out of the box, as far as I can tell, but I was able to work around it using the app-module-path npm package in our mocha helper file:

import path from 'path';
import AppModulePath from 'app-module-path';

const jsPath = path.normalize(__dirname + '/../../app/assets/');
const vendorPath = path.normalize(__dirname + '/../../vendor/assets/javascripts');

AppModulePath.addPath(jsPath);
AppModulePath.addPath(vendorPath);

I'm not aware of a way to get around it using browserify, so you may be stuck with modifying NODE_PATH.

And to make eslint-plugin-import work correctly, I added the following to my .eslintrc.yml:

settings:
  import/resolver:
    node:
      moduleDirectory: [app/assets, node_modules]

[ljharb]

@jazzdragon because it makes it impossible to look at your code and know where something is being required from. node is not going to implement support for it with ES modules also, so you'll be well advised to migrate off of it. It's not important what makes sense to you - it's important what will make sense to every node/JS developer, and that's "the node require algorithm".

@mockdeep it's not officially deprecated in the sense that they won't be removing it for require, but it's deprecated in the sense that ES modules will never support it, and it's widely considered a very bad practice in the node/JS community.

The fact that tools like jest and webpack give you the ability to approximate NODE_PATH doesn't make it a good pattern; it just means those tools want to cater to the users who want that pattern (whether it's a good one or not). The proper solution, if you dislike chains of ../, is to extract things into separate npm-installed modules; things that are bad (overly large projects) should look bad, otherwise you're just sweeping problems under the rug.

[mockdeep]

@ljharb why is it better to pull things out into npm installed modules? The only issue I see mentioned is the possible performance of it, but I haven't seen any benchmarks to convince me that there's a difference worth worrying about, nor have we had any performance issues related to package loading. Having to pull things out into npm modules just sounds like a pain in the butt, though.

[ljharb]

@mockdeep it encourages the proper encapsulation. It's got nothing to do with performance (that's almost never important when programming), it's about maintainability, readability, encapsulation, reusability, etc. In general, it's easy to consolidate but hard to separate, so whenever you get the opportunity to separate things, take it.

[msikma]

It's very striking how this thing just keeps coming up time and again, though. Every once in a while someone mentions NODE_PATH and the need to do imports relative to the current module, and every time it's the same story: best practices, small modules, don't mess with Node's way of doing things.

Regardless of what any one of us might think, to me this does not indicate simply a failure to communicate how Node works, but a disagreement among a significant amount of Node users on how it should work in the first place. The numerous iffy workarounds are testament to this. I don't think it's correct to say it's "widely considered a bad practice" because this issue just refuses to die.

In my view, the core problem is that the Node module resolution algorithm doesn't support import paths relative to the current package—only to the current file, to the filesystem root, or to any file (not even package, necessarily) inside node_modules. If it were possible for a file in package myPackage to import myPackage/components/file.js, this whole discussion could probably be avoided. That's how Python works and probably loads of other languages. It's very strange that you can have import statements that start with any package name, except your own package, and I'm curious why it was designed with this obvious limitation to begin with.

[mockdeep]

@ljharb while I can certainly get on board with the idea that good encapsulation is important, it seems like there are a lot of ways to encourage it without taking on the additional burden of maintaining a bunch of separate npm packages. It seems like we're exchanging one source of pain for another, not to mention using paths as a source of pressure towards that goal seems a little indirect and easy to draw the wrong conclusions from. In our case we encourage better encapsulation explicitly via our linters and having a strong culture around code review.