Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Node.js imports field is not supported #327

Open
pcdevil opened this issue Nov 24, 2024 · 3 comments
Open

Node.js imports field is not supported #327

pcdevil opened this issue Nov 24, 2024 · 3 comments

Comments

@pcdevil
Copy link

pcdevil commented Nov 24, 2024

Imports fields is not supported

Overview

Short description: resolve fails to handle Node.js' imports field style imports.

Steps to reproduce:

  1. Add a simple entry to imports fields to package.json:
    +  "imports": {
    +    "#src/*": "./src/*"
    +  },
  2. Create a src/module-1.js file
  3. Call resolve (async or sync) with #src/module-1.js argument

Expected behaviour: resolve() returns the full path of the module

Actual behaviour: resolve() throws an error

Minimal reproducible example: I created the resolve-nodejs-import StackBlitz project to showcase the issue with some guiding. Running npm start should execute the script and show the behaviour of resolve() in various circumstances.

Context

I'm using the resolve package as a transitive dependency of eslint-plugin-import for a small project I created, wordoftheday.

This project is very minimal and doesn't utilise any bundler but instead relies on Node.js' module resolution and imports field. By this, the imports are non-relative paths with TypeScript style import aliases. For example the src/lib/word-resolver.mjs file has the following dependencies (at L3-6):

import { UndefinedConfigError, config } from '#src/lib/config.mjs';
import { getLogger } from '#src/util/logger.mjs';
import { NamedError } from '#src/util/named-error.mjs';
import { request } from '#src/util/request.mjs';

This works just fine on run, I don't experience any side-effects by calling pnpm start.


I decided to upgrade ESLint to v9 and after tinkering the config file, I received a lot of errors from import's no-unresolved rule when I ran eslint.

After some investigation I realised the issue lies deeper at resolve, as it doesn't unfold module names starting with a #. I believe the issue is at the check where the module's nature is determined:

  • sync:

    resolve/lib/sync.js

    Lines 101 to 111 in fd788d9

    if ((/^(?:\.\.?(?:\/|$)|\/|([A-Za-z]:)?[/\\])/).test(x)) {
    var res = path.resolve(absoluteStart, x);
    if (x === '.' || x === '..' || x.slice(-1) === '/') res += '/';
    var m = loadAsFileSync(res) || loadAsDirectorySync(res);
    if (m) return maybeRealpathSync(realpathSync, m, opts);
    } else if (includeCoreModules && isCore(x)) {
    return x;
    } else {
    var n = loadNodeModulesSync(x, absoluteStart);
    if (n) return maybeRealpathSync(realpathSync, n, opts);
    }
  • async:

    resolve/lib/async.js

    Lines 141 to 164 in fd788d9

    if ((/^(?:\.\.?(?:\/|$)|\/|([A-Za-z]:)?[/\\])/).test(x)) {
    res = path.resolve(basedir, x);
    if (x === '.' || x === '..' || x.slice(-1) === '/') res += '/';
    if ((/\/$/).test(x) && res === basedir) {
    loadAsDirectory(res, opts.package, onfile);
    } else loadAsFile(res, opts.package, onfile);
    } else if (includeCoreModules && isCore(x)) {
    return cb(null, x);
    } else loadNodeModules(x, basedir, function (err, n, pkg) {
    if (err) cb(err);
    else if (n) {
    return maybeRealpath(realpath, n, opts, function (err, realN) {
    if (err) {
    cb(err);
    } else {
    cb(null, realN, pkg);
    }
    });
    } else {
    var moduleError = new Error("Cannot find module '" + x + "' from '" + parent + "'");
    moduleError.code = 'MODULE_NOT_FOUND';
    cb(moduleError);
    }
    });

Closing thoughts

The issue is not related to ESLint v9, but I mistakenly didn't include eslint-plugin-import to my .eslintrc.json's extends field (at L7-10) and only realised the mistake on update.

I can understand if you believe it's out of the scope of this package, although I'd think the imports field feature would be supported. In that case a possible workaround for me is to wrap eslint-import-resolver-node and transform imports starting with # to project relative paths.

However, if you believe it's worth fixing it, I'd also volunteer to provide a change-set for review!

@ljharb
Copy link
Member

ljharb commented Nov 24, 2024

We also don’t yet support the exports field, which is a prerequisite for supporting the imports field.

It is definitely in scope of this package, but it is a massive amount of work.

@pcdevil
Copy link
Author

pcdevil commented Nov 30, 2024

I didn't see exports as a prerequisite for this feature but I'm not that much familiar with these features of Node.js to know better so I take your word for it!

as I'm reading the Node.js documentation and I agree it's not a small work, there are a lot of cases to be handled: string vs object value, subpaths, globs, conditional export (with nesting), and with imports even other packages too... 🤔

I don't want to jump on it without planning (or even, posting a huge change-set and call it a day that I made it, accept it), so the only appropiate way I see to talk about it and ask advise from you.

If it's in your interest, how can I contribute to this feature?

@ljharb
Copy link
Member

ljharb commented Nov 30, 2024

It is, because the resolution of the imports field uses the exports field’s values.

The place I’m still working on it is in https://github.com/ljharb/list-exports, which has lots of text fixtures. Feel free to take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants