resolve icon indicating copy to clipboard operation
resolve copied to clipboard

Node.js imports field is not supported

Open pcdevil opened this issue 1 year ago • 3 comments

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:
    https://github.com/browserify/resolve/blob/fd788d94d037e32d4f4be948e2f7e15f6981f004/lib/sync.js#L101-L111
  • async:
    https://github.com/browserify/resolve/blob/fd788d94d037e32d4f4be948e2f7e15f6981f004/lib/async.js#L141-L164

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!

pcdevil avatar Nov 24 '24 13:11 pcdevil

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.

ljharb avatar Nov 24 '24 17:11 ljharb

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?

pcdevil avatar Nov 30 '24 11:11 pcdevil

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.

ljharb avatar Nov 30 '24 17:11 ljharb