utils: Uses createRequireFromPath to resolve loaders

[arcanis]

The current logic uses require.resolve in order to locate the resolver modules. The problem is that require.resolve semantically operates based on the currently evaluated module. That makes the resolution incorrect, as the loaders aren't dependencies of eslint-module-utils. It probably works thanks to the hoisting, but hoisting is on its way out and such problems will start to reveal themselves.

To fix that, the resolution simply needs to use createRequireFromPath so that the loaders are located based on the dependencies of the package that contains the source file being linted - which I think was the intent.

[coveralls]

Coverage increased (+0.06%) to 96.258% when pulling fb0cbeb on arcanis:patch-1 into 16ae652 on benmosher:master.

[coveralls]

Coverage increased (+0.1%) to 96.323% when pulling 64f0452 on arcanis:patch-1 into 16ae652 on benmosher:master.

[coveralls]

Coverage increased (+0.1%) to 96.323% when pulling 64f0452 on arcanis:patch-1 into 16ae652 on benmosher:master.

[coveralls]

Coverage increased (+0.1%) to 96.323% when pulling 64f0452 on arcanis:patch-1 into 16ae652 on benmosher:master.

[coveralls]

Coverage increased (+0.1%) to 96.323% when pulling 64f0452 on arcanis:patch-1 into 16ae652 on benmosher:master.

[ljharb]

Is there a way to have a test for this?

[arcanis]

Added a test that fails on master and passes on this branch 👍

[johanneswilm]

This seems to fail whenever one doesn't follow the standard directory structure of eslint6 and because this was released as a patch version it's also pulled in by users of eslint5.

[johanneswilm]

I wonder what the need was to merge this in. Was ot broken for anyone before this was merged? If not, I'd suggest undoing this as there is then no benefit to anyone and it only creates problems for some users.
In general there should be an eslint option to specify where to resolve modules, resolvers, parsers from. Otherwise it just doesn't work for the workflows of many.

[arcanis]

Was ot broken for anyone before this was merged? If not, I'd suggest undoing this as there is then no benefit to anyone and it only creates problems for some users.

Well, as you can guess it wouldn't have been merged unless it had been fixing something.

Without going into the specific, the loaders were loaded in a rule-breaking way that would have caused a lot of pain in a few weeks. We certainly can find a smoother migration path, but reverting this diff won't go into our users' best interests.

[johanneswilm]

Could this be applied in a way so that it doesn't get installed for users of eslint5 and current versions of eslint6? For example by not just marking it a patch? Or what the benefit be for them (me)?

[DavideDaniel]

As yarn 2 gains adoption this is becoming a real issue at our company. We are currently debating removing this plugin which would be unfortunate as it's provided immense value. What can we do to help address this?

[ljharb]

@DavideDaniel have you fully updated eslint-plugin-import and eslint-module-utils? As far as I'm aware, this issue is fixed for yarn v2.

[DavideDaniel]

@DavideDaniel have you fully updated eslint-plugin-import and eslint-module-utils? As far as I'm aware, this issue is fixed for yarn v2.

Super delayed but we addressed the issue via this: yarnpkg/berry#1427 (comment)

[mainrs]

It still doesn't work correctly without the workaround posted above. The above one is just a workaround for packages that forgot to declare a dependency. It just tells yarn to use the package from your own devDependencies instead.

I have fully updated the packages and still experience the issue. However, I am not entirely sure how this would have to be solved. My guess would be to add a peerDependency that matches the version from the locally committed version inside the repository (so 0.3.4).