feat: support ESM subpath imports

[fi3ework]

Description

resolve #7385

Vite now uses https://github.com/lukeed/resolve.exports to determine the target file of exports filed. According to the node.js spec and this lukeed/resolve.exports#14. This PR made a little fork of resolve.exports based on https://gist.github.com/okikio/3f07571c7707dc6e6eb4906b951c6bc3 to implement resolve.imports. We can remove the code from Vite until resovle.exports supports imports filed.

Additional context

https://github.com/fi3ework/vite/blob/7acab6a9db3aecb7b11d955598c1b48d58f7956b/packages/vite/src/node/plugins/resolve.ts#L152-L154

This PR re-assign id at the beginning for imports, I'm not sure is it the best way to do this or we could add another plugin like pre-alias? 🤔

2. The test cases referenced evanw/esbuild@62a3943

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines.
• Read the Pull Request Guidelines and follow the Commit Convention.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Ideally, include relevant tests that fail without this PR but pass with it.

[wight554]

Any blockers for this one?

[shirotech]

Awesome job, been looking for a way to do this, hope will be merged soon :)

[seivan]

Maybe this isn't the right place, but if you're using package.json (assuming I got it right) for storing the module maps, that means everything else would have to infer from it.

Wouldn't that be an issue, if you would have to repeat it again for TSConfig?

Edit: Also, amazing work 🎉

[benmccann]

it looks like this PR needs a rebase

[bluwy]

Update: From the last team meeting, we discussed that this is a feature we want, but we would want to use a centralized standard package to resolve subpath imports instead of a fork. We're currently coordinating with @lukeed to get this done, and after that we can update this PR to use it.

[brupelo]

Hi guys, vite newcomer over here, first of all, let me tell you how amazing vite project it's, in our case we've been already suffering some issue with a large CRA app that has hundreds of components and I'd really like to give it a shot to vite eventually but there are few blockers I'd need to solve before reaching that point and one of them being subpath imports support by the bundler.

I've tried to see if subpath imports was already working with latest vite atm ([email protected]) but it seems is not, then I just found this PR.

Attaching a little reproducer in case it may help somehow test_storybook_subpath_imports.zip

When uncommenting line 2 of

You should get a crash like

Also, I guess this feature should be able to handle properly subpath imports containing non-js/jsx code, right? ie: assets such as json, svg, css, ...

ie: import reactLogo from '#assets/react.svg'`

[fi3ework]

It seems like https://github.com/privatenumber/resolve-pkg-maps is an alternative to https://github.com/lukeed/resolve.exports. Maybe we could move to resolve-pkg-maps as resolve.exports hasn't been maintained for a while. There is some difference described in https://github.com/privatenumber/resolve-pkg-maps#how-is-it-different-from-resolveexports, not sure these will be the breaking change. 🤔

[patak-dev]

@fi3ework I think it is a good idea. IIRC @aleclarson also started a fork of lukeed/resolve.exports. Alec, it would interesting to hear your opinion here about @privatenumber new library.

[aleclarson]

We already have #10504 and #10929 to integrate @alloc/resolve.exports with Vite. I think there's value in using a solution built with Vite in mind, instead of a general purpose solution. Also, I think merging my resolve.exports fork into the Vite org would be worth while, so we don't have to rely on a third party for bug fixes to this critical piece of Vite's path resolution going forward.

[lukeed]

I like how “critical” resolve.exports is to vite & how much chatter there has been about it here (and elsewhere) yet there are no PRs from the work done here to fix the few bugs & add the really easy imports support. Nor have there been any offers to fund its maintenance or continued support. Not sure if it’s because people continue to assume/expect me to be a full-time OSS maintainer for free or because they see an opportunity to fork/copy-edit a new variant to siphon some “popularity” … either way, OSS has become such a game and I’m not here for it. Vite/Vue is sitting on a pile of cash & should be setting a clear example, while it has the attention, for how an open source ecosystem can grow and thrive sustainably instead of relying on an exorbitant amount of free labor.

[yyx990803]

Hey @lukeed, first of all, thank you for your work on resolve.exports. Please note that we would never consider it your responsibility to implement anything that Vite needs specifically. It is your package, we do not expect anything more than what you've already done.

On the other hand, I think your accusation of "relying on an exorbitant amount of free labor" is misdirected. Vite is not making "a pile of cash" by any means. Our sponsorships barely generate enough to cover the salary of an entry level engineer, and we try to disperse it among team members who make consistent, direct contribution to Vite itself. Also do note I actually work on OSS fulltime, sponsor a lot of other devs in the Vue / Vite ecosystem, and need to make a living. Funding dependencies will be nice and we may consider it in the future, but I don't think that should be considered a responsibility.

We used resolve.exports because it aligned with what Vite needed. I think a fork / replacement is considered mainly because (1) assumed maintenance inactivity on resolve.exports. and (2) the possibility of needing more, maybe Vite-specific logic in the future.

The part I think we should have done better here is better communication with you from the beginning. I also 100% agree that we should've submitted a PR with the desired feature before moving towards a fork. Would you be open to consider that if we did?

[aleclarson]

I like how “critical” resolve.exports is to vite & how much chatter there has been about it here (and elsewhere) yet there are no PRs from the work done here to fix the few bugs & add the really easy imports support. Nor have there been any offers to fund its maintenance or continued support. Not sure if it’s because people continue to assume/expect me to be a full-time OSS maintainer for free or because they see an opportunity to fork/copy-edit a new variant to siphon some “popularity” … either way, OSS has become such a game and I’m not here for it. Vite/Vue is sitting on a pile of cash & should be setting a clear example, while it has the attention, for how an open source ecosystem can grow and thrive sustainably instead of relying on an exorbitant amount of free labor.

@lukeed I feel a need to defend myself here. My fork of resolve.exports was strictly out of necessity. I just don't have the time or desire to jump through the hoops needed to get a PR merged into your library. I mean no offense. It really doesn't matter whether your library, a fork of it, or another library entirely is used for imports/exports mapping. What matters is that Vite users get this feature as soon as possible. I've done as much as I can to provide for that within my means. Finally, might I ask that you refrain from posting criticism openly before reaching out in private.

[lukeed]

We all know it’s much easier to patch fixes than it is to set up a new repo with all new tests, docs, and comparisons. And there’s nothing Vite needs that the ”general public” doesn’t also need. So I don’t understand that argument.

@yyx990803 theres clearly been motions towards a desired outcome already. Not that it was intentional or that you had any part in it, but the work has already been done and I’m not motivated by others’ desire for exposure/popularity/downloads. I have real bills to pay now and so OSS dramatically fell off the wayside. Congrats to you & others who’ve made it financially feasible, but I’ve invested 60+ hour weeks into OSS for years now and can’t justify “exposure” and “networking opportunities” for the time spent any more. I don’t mean this to be a personal soap opera hour but I’ve given away $500k equivalence of consulting hours over the years for little return. I don’t care about resolve.exports but it seemed like a great opportunity to point out the issues in OSS that are/were present here & I feel like Vite is in a great place to start changing things. Myself removed.

[dominikg]

It really doesn't matter whether your library, a fork of it, or another library entirely is used for imports/exports mapping. What matters is that Vite users get this feature as soon as possible.

Imho it does matter to the node/web ecosystem at large. Having multiple packages doing the same thing in often subtly different ways isn't great. Fragmentation and all. This would hurt vite in the long run too as it has to deal with the fallout of any incompatibilities that arise out of this mess.

Vite has been great at working together with some upstream packages it depends on, rollup being the most prominent example here. Extending that pattern to other packages would benefit everyone. Sharing the burden of maintaining, using the public outreach it has to match up people who want to help with projects that need help etc. magic-string has been another example at this.

So please lets do OSS cooperation instead of fragmenting things even more 🥺

[lukeed]

resolve.exports@next just went out for early preview. It includes "imports" support via an imports function. The resolve function still exists & acts the same, but it's actually a convenience helper that reroutes to r.imports() or r.exports() depending on the target specifier it received. You can look over the API docs for more info.

Planning to release 2.0 stable this Monday or Tuesday if all looks to be ok

[lukeed]

Sorry, forgot to come back here with an update – the 2.0 was released a day after my last comment.

[benmccann]

Thanks @lukeed! It looks like Vite was upgraded to 2.0 here: #11712. We'll need another PR to leverage the imports support

[lukeed]

Ah cool! :)

[fi3ework]

Have updated this PR with resovled.exports 2.0. Looking for some code reviews. 👀

[bluwy]

LGTM. Great tests!

[bluwy]

Note to self. exports only work for root package.json, but node decided that imports doesn't need to and works for the closest package.json, so this is fine. Stackblitz

[lehni]

Amazing! thank you @fi3ework for the hard work on this!

[lehni]

Having read through the PR finally, I just realized that a massive thank you needs to go to @lukeed. I recognize the burden of maintaining OSS, and I appreciate all the work you're putting in for the rest of us!

[lukeed]

Thanks :) big thanks to @fi3ework for the integration here & to @bluwy for the all-around encouragement/motivation

[tsujp]

Looks like this has been included in Vite 4.2.0-beta.1 however unless I am using this incorrectly this doesn't appear to work in-situ. It does resolve a subpath import correctly but it cannot be used with a real world codebase because import-analysis does not understand what to do with a subpath import when given it and so it resolves to null instead which returns:

PM [vite] Internal server error: Failed to resolve import "#src/routes/app_router" from "src/main.tsx". Does the file exist?
Plugin: vite:import-analysis
File: /path/to/codebase/apps/www/src/main.tsx:1:26
4  |  import { $$registry as _$$registry } from "solid-refresh";
5  |  const _REGISTRY = _$$registry();
6  |  import { AppRouter } from '#src/routes/app_router';
   |                             ^

Annotated with a bunch of console.log statements when trying to debug this I can see the following:

(1) asked to resolve: #src/routes/app_router
(2) problem is here: null
(3) asked to resolve: #src/routes/app_router resolved to: [ './src/routes/app_router.js' ]

(1) is from within vite:import-analysis right before it switches on if (!resolved).

console.log('asked to resolve:', url)
const resolved = await this.resolve(url, importerFile);
                if (!resolved) {

(2) is from within that if statement.

(3) is from within vite:resolve.

import-analysis calls some resolve method (I'm working with the post-bundled source here so there are umpteen definitions for that function) which doesn't know what to do hence the failure.

I'll try actually find this in the normal codebase before it's been bundled.

Minimum reproducible example here: https://stackblitz.com/edit/github-c6tkmr-1eubsp?file=src/index.tsx

[privatenumber]

Looks like it's because your import map entry is:

"#src/*": "./src/*.js"

but you're using a TypeScript import:

import { Foo } from '#src/foo';

to resolve ./src/foo.tsx with a .js extension.

Vite will need a TypeScript compliant resolver for this. I'm actually working on one for tsx because it has the same issues.

[tsujp]

Looks like it's because your import map entry is:

"#src/*": "./src/*.js"

but you're using a TypeScript import:

import { Foo } from '#src/foo';

to resolve ./src/foo.tsx with a .js extension.

AFAIK that is correct because Typescript does not rewrite import paths (even though they actually do but that's a whole different discussion and a contentious one within the Typescript repo). Specifically here they do not rewrite the extensions. Even without an import map the only way to use that component would be to import it with the .js extension even though on-disk it is a .tsx component because after compilation it will be a .js file and... Typescript does not rewrite extensions.

Vite will need a TypeScript compliant resolver for this. I'm actually working on one for tsx because it has the same issues.

Ah yes I hadn't considered that Typescript will be resolving the import map in a manner consistent unto itself given the whole import-path-rewriting and that Vite may not be aware of that. As much as things like Vite have 100% improved the JS ecosystem it's (said ecosystem) still such a pot of spaghetti.

What's strange (to me at least) is that adding a very simple alias to the Vite configuration like the one below fixes this. There's no explicit extension changing here, just a simple find-replace

resolve: {
   alias: [
      {
         find: /^#src/,
         replacement: './src',
      }
}

[CMCDragonkai]

Question as far as I can tell, this is not working atm for vite 5.2.11. I still had to provide a tsconfigPaths() plugin to my vite.config.ts file.