fix vite dep optimizer

[patricklx]

tests: https://github.com/embroider-build/embroider/pull/1876/files#diff-3e325354530b11c4e6b3ab941d93a8fe2ceb26087f9c13856c9d8684364a3fb3R62

• all deps are optimized
• all optimized chunks are used
• new deps are optimized
• addon rebuilds are newly optimized

dep optimizer has multiple phases

• Scanning, which uses the vite plugins and configured rollup plugins .
• pre bundle, does not use configured rollup plugins.

This is the order

• inital scan
• initial scan must provide correct import specifier to vite import analysis plugin, this is probably because we cannot use vite own alias handling
• pre bundling
• vite starts and gets requests
• vite checks if some need optimization
• rules:

1. Specifier must be bare import
2. Importer cannot be in node modules
3. Needs to be node_resolveable, so follow node resolution without additional plugin resolution

• if a new dep is detected, vite schedules a new scan

Currently this is broken because of 2 things. app is in rewritten-app. It's actually easy to fix by changing to importer to the original app. Only needed when resolving bare imports.

The more difficult one is rewritten-packages. They must be node resolvable from original app. They are not currently.
I suggest to create a link from @embroider/rewritten-packages to the one inside . embroider and rewrite specifiers accordingly.

It currently looks like it's working, because the initial scan detects the deps. But any new dep added later will not be optimized.

fixes:

• rewrite rewritten-app to original app location
• link rewritten-package to be node resolvable
• esbuild scan needs to pass renamed modules and appjsmatch to vite import analysis

[ef4]

I suggest to create a link from @embroider/rewritten-packages to the one inside

This is how things used to work. Doing it correctly requires more than just links to the rewritten packages, in general it requires you to rewrite the entire node_modules graph in order to make every package see all the correct dependencies.

And I don't think it's necessary. I think that from the perspective of the rewritten-app, the rewritten-packages don't appear to be inside node_modules. But when we stop rewriting the app, they will.

[patricklx]

I suggest to create a link from @embroider/rewritten-packages to the one inside

This is how things used to work. Doing it correctly requires more than just links to the rewritten packages, in general it requires you to rewrite the entire node_modules graph in order to make every package see all the correct dependencies.

And I don't think it's necessary. I think that from the perspective of the rewritten-app, the rewritten-packages don't appear to be inside node_modules. But when we stop rewriting the app, they will.

The problem is that vite needs a bare import and the importer cannot be in node modules.
What I'm aiming to do here is to create a resolved bare specifier.
So instead of having ./node_modules/.embroider/rewritten-packages/pgk1/node_mod/pkg/my-file.js
We have
@embroider/rewritten-packages/pgk1/node_mod/pkg/my-file.js
And things will work

What we are currently doing for resolving rewritten-packages is doing a rehome inside the rewritten-packages, onto moved-target.js https://github.com/embroider-build/embroider/blob/main/packages/core/src/module-resolver.ts#L954

And here is how vite checks if it should optimise:

• Specifier must be bare import
• Importer cannot be in node modules
• Needs to be node_resolveable, so follow node resolution without additional plugin resolution

When we are doing a rehome, the importer is in node_modules... So rhat doesn't work

https://github.com/vitejs/vite/blob/6c323d5b3ab3cdf81d21bbe965ed3c36aa7f0589/packages/vite/src/node/plugins/resolve.ts#L868
and
https://github.com/vitejs/vite/blob/6c323d5b3ab3cdf81d21bbe965ed3c36aa7f0589/packages/vite/src/node/plugins/resolve.ts#L353

So it's not a problem of rewritten-app.

[patricklx]

ignored should fallback resolve

[patricklx]

workaround for fromFile pointing at /app/tests/package.json and vite/deps

[mansona]

I just discussed this on the Embroider Triage meeting with @patricklx and I'm going to do my best to summarise the situation here:

Firstly we started to spec out a way to test if a dependency was optimised correctly in the office hours last week. A good first draft of that work can be seen in #1880 where we run vite in dev mode and try to "audit" the app across HTTP. This will allow us to make sure that the dependencies we want to check are coming from the right place (i.e. from the .vite/deps folder)

This work is blocked on the refactor that myself and @ef4 are pairing on to allow audits to work across HTTP (and async) which hopefully will land this week.

Once the audit refactor lands we will make sure to provide a suite of tests that show the problems that @patricklx is identifying in this PR (and Discord discussions), we can then check those tests against other work that is going on (e.g. #1779 ) to make sure that they are still failing and we can fix accordingly 👍