eslint-plugin-import icon indicating copy to clipboard operation
eslint-plugin-import copied to clipboard

Add support for require.resolve

Open vikr01 opened this issue 6 years ago • 8 comments

Closes #585

Related to discussion in #1035 (link to comment).

So far I've only added require.resolve for commonjs with no-unresolved, let me know if I should add it anywhere else.

vikr01 avatar Oct 20 '18 23:10 vikr01

Coverage Status

Coverage increased (+0.09%) to 97.349% when pulling 497ead05001478e1c6d26ff1b3cec5dfeb42d832 on vikr01:feature/require-resolve into b4a2f11fcacc6b2f048da4b29cfc896e682f17d1 on benmosher:master.

coveralls avatar Oct 20 '18 23:10 coveralls

@ljharb was thinking the same thing, yeah I'll make a version using an option

vikr01 avatar Oct 21 '18 04:10 vikr01

Closing in favor of #1217

vikr01 avatar Oct 24 '18 08:10 vikr01

I'm going to keep the two in sync; orphaned PRs clutter the log.

ljharb avatar Oct 24 '18 08:10 ljharb

@ljharb why is this a breaking change if require.resolve is part of CommonJS?

feels semver minor to me, in that it is sortof a bugfix, but broad enough that it qualifies as an added feature.

I'd prefer this over require-ing (pun intended) users to discover and enable this behavior.

benmosher avatar Oct 25 '18 11:10 benmosher

@benmosher There may be some who are using require.resolve on files generated at build time (instead of path.join/path.resolve).

vikr01 avatar Oct 25 '18 18:10 vikr01

fair enough, I guess can just try to make the note to bump it to on-by-default in v3.

benmosher avatar Oct 26 '18 23:10 benmosher

@benmosher new warnings are almost always a breaking change.

ljharb avatar Oct 26 '18 23:10 ljharb