Custom resolver path

[le0nik]

With this PR users will be able to require resolvers:

1. With absolute paths (/Volumes/..., user can include them as computed property in .eslintrc.js config)
2. With paths, relative to closest package.json (./eslint-plugins/webpack-resolver)
3. With custom npm modules names (@myorg/webpack-resolver)
4. With conventional npm modules names (webpack -> eslint-import-resolver-webpack)

[lukeapage]

Hi,

this is similiar to my #312 and does alot more. But it doesn't allow a project relative path which I needed (absolute paths are useless for shared projects)

[lukeapage]

ah sorry I understand - the user can calculate the absolute path in the config and pass that. I guess its because we don't know the path of the .eslintrc file ? I just went with resolving to cwd

[le0nik]

@lukeapage What do you mean? See 2). You can pass relative path, e.g. ./my-resolver-plugin. It will calculate it from your root project directory.

[le0nik]

@lukeapage I saw your PR. I don't think it works at all, to be honest :/.

On this line: https://github.com/benmosher/eslint-plugin-import/pull/312/files#diff-2bd490f6c0bd766af6e36e61189196c8R148 you check if the path is absolute(which would be for example /Volumes/my-project/...). If true, this path would be appended to eslint-import-resolver- and become eslint-import-resolver-/Volumes/my-project/..., which isn't really what you want.

I decided to go another way with this PR and add some more options(my requirement is to also allow local resolvers and private resolver packages).

[lukeapage]

Ah i see how (2) works sorry, i replied first thing in the morning, must have still been asleep.

Yes your pr is superior, had just missed (2).

[benmosher]

Sweet! I pulled requireResolver out to a function with exactly this in mind. 😁 Hadn't gotten around to it.

I will look more closely when I get a chance to understand why tests are failing,

[le0nik]

@benmosher The problem with failing tests seems to be within this function, that doesn't return context.report, where as resolve expects context.report to be a function.

[benmosher]

That must mean some test is failing to load a resolver and it's trying to report it in the catch handler.

[le0nik]

@benmosher It was my fault. It's fixed now, but one test still doesn't pass on Node 0.12. I can't reproduce it on my machine with Node 0.12. If you have an idea on how to fix it, i'll gladly do it.

[benmosher]

Agh, no worries, it's not you, it's Travis. I need to bump up the timeouts or something.

I will double-check before I merge, but it's probably good to go. 😎

[le0nik]

Ok, thanks! After you check this, tell me if you need me to squash it and I'll do it.

[le0nik]

Is there anything I can do to get this PR merged?

[lukeapage]

+1 am waiting for a release with this in before I can use this plugin.

[benmosher]

Sorry, got this confused with #320 and had parked it mentally. I think it's good, would be better if it had tests but the code looks good.

Can you

• rebase against master
• update the change log (probably to the Added section)
• add a note to the README resolvers section that denotes the ways that resolvers can be referenced

Also, if you could

• update it to use pkg-dir's sync function instead of find-root

that would be ideal as pkg-dir shares a dep with pkg-up (which is already a dependency), and does roughly the same thing. I am planning to do the same with the find-root reference in the Webpack resolver at some point.

[leonid-bauxy]

@benmosher done!

[lukeapage]

@benmosher any chance of getting this merged soon now your todo's are done?

[benmosher]

Yeah, should be good. I need to make another run through the changes but I think it will be good.

[benmosher]

@le0nik what motivated this change?

[le0nik]

As i remember, it was due to this failing test after merge of master.

Ah, i see what you mean. My mistake. The test affected just the line above, not this one. I shouldn't have changed this.