fix: trigger double resolving for wrapped ids

[patak-dev]

Description

Fixes #13404

I don't know if this fix is really needed. At least if it should count as a regression for the reproduction provided. The reproduction works if the alias is resolved instead of being pointed to "@vite-test/core/src":

  resolve: {
    alias: {
      "@core": path.resolve("../core/src/"),
    },
  },

But we have some examples of aliases like this one in our playground (for example { find: 'dep', replacement: '@vitejs/test-resolve-linked' }).

I think these cases could lead to duplicated modules in the browser. The path is rewritten as /@id/@vite-test/core/src/... and later on the browser could load the same module under a different URL (if it is resolved without starting from the @core alias).

I think it is safer to still apply this as a patch, and break it in Vite 5 if we consider that should be the path forward.

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[patak-dev]

Added a test case that will fail during build. This was already failing in Vite 4.2, so it isn't a regression. I think we maybe we should close both this PR and the issue.

[sapphi-red]

Ah, sorry, I noticed that this shouldn't work. The current behavior is correct.
@vite-test/core/src/utils is not possible to resolve inside @vite-test/core. To make this work, @vite-test/core needs to have exports field. Then, the self-referencing feature would be used.

[bluwy]

Also +1 for working as intended since the build was also failing in the repro.

[patak-dev]

Ah, sorry, I noticed that this shouldn't work. The current behavior is correct. @vite-test/core/src/utils is not possible to resolve inside @vite-test/core. To make this work, @vite-test/core needs to have exports field. Then, the self-referencing feature would be used.

I don't think having exports would be enough here. Did you see the repro working when adding it? I pushed exports in the repro and build is still failing (and dev fails too without this PR). Maybe there is something wrong on the test case?
I'll still close this PR later because we never should need to double resolve, @core should be replaced by the alias plugin, and then resolve should be called by the plugin with the updated id.

[sapphi-red]

Ummm, it seems there's a different bug here. The self-referencing feature doesn't work with Vite when link: or workspace: is used.
If I add the following exports field to the @vite-test/core's package.json and replace "@vite-test/core": "workspace" with "@vite-test/core": "file:../core" in the @vite-test/web's package.json, it works.

  "exports": {
    ".": "./src/index.ts",
    "./*": "./*/index.ts"
  }

https://stackblitz.com/edit/github-yylndb?file=packages%2Fcore%2Fpackage.json,packages%2Fweb%2Fpackage.json

[sapphi-red]

I'll close this one as the original issue was working as intended. 🙏

The bug I described above was already tracked at #9731.