Properly resolve base config #545
Conversation
Signed-off-by: Richie Bendall <richiebendall@gmail.com>
| @@ -52,6 +52,9 @@ resolveFrom.silent = (moduleId, fromDirectory) => { | |||
| } catch { } | |||
| }; | |||
|
|
|||
| // TODO: Use `resolveModule(normalizePackageName(name), import.meta.url);` when moving to ESM then to `import.meta.resolve(normalizePackageName(name))` when supported | |||
| const resolveLocalConfig = name => resolveModule(normalizePackageName(name, 'eslint-config'), require.main.filename); | |||
There was a problem hiding this comment.
Wouldn't it be more correct to use path.resolve('..', __dirname) instead of require.main.filename?
I'm not sure how resolveModule works, but doesn't resolveModule(normalizePackageName(name), import.meta.url); also need to search from the parent directory of import.meta.url? Regarding the code comment.
There was a problem hiding this comment.
doesn't resolveModule(normalizePackageName(name), import.meta.url); also need to search from the parent directory of import.meta.url?
resolveModule just wraps require("module").createRequire(path).resolve(name) which is the same code that is recommended to be used in a related StackOverflow question.
Wouldn't it be more correct to use path.resolve('..', __dirname) instead of require.main.filename?
Better still, we could just use name => require.resolve(normalizePackageName(name)).
CommonJS and ESM support is mutually exclusive for the code so I left upgrade notes instead.
Automatically testing for the issue mentioned in the original report is unfeasible so it will need to be tested for manually.
Fixes: #543