Core: Restore /preview etc package exports; return unresolved path from presets.

[tmeasday]

Issue:

Chromatic regressions introduced in #19007, ultimately due to many addons not working in example apps.

The root issue that was being worked around was that if a package has an exports field, Vite can only import an entry there. i.e. it can import('@storybook/addon-backgrounds/preview') not import('@storybook/addon-backgrounds/dist/preview.js'), which is what the presets code was resolving it to.

What I did

In the presets code, try resolving an import path, but return the import path unmodified, so the builder can import it itself.

NOTE: This means that import paths (e.g. previewEntries) will be of the form '@storybook/addon-backgrounds/preview' rather than /Users/xyz/..../node_modules/@storybook/addon-backgrounds/preview. Is this a problem @ndelangen? If so we could "half resolve" the import (ie. absolutize the import path).

How to test

• Run example app (in both Vite and Webpack), check the addons, a11y, etc addons are working.
• Chromatic changes should be gone on this PR.

[shilman]

LGTM. @ndelangen we should probably figure out how to clean this up somehow -- maybe in Vite, which was the reason for the change in the first place?

[IanVS]

The reason vite is importing the files with .js is that's what we get from await presets.apply('config', [], options);

[tmeasday]

@ndelangen -- @shilman and I reworked this, and we are happy with this solution now, it resolves (haha) everything, apart from one possible wrinkle (see above):

NOTE: This means that import paths (e.g. previewEntries) will be of the form '@storybook/addon-backgrounds/preview' rather than /Users/xyz/..../node_modules/@storybook/addon-backgrounds/preview. Is this a problem @ndelangen? If so we could "half resolve" the import (ie. absolutize the import path).

[ndelangen]

bingo! this is great!

when I opened this PR this morning I instantly had the realization that this was the problem, and then 1 click later, you've implemented this exactly like that.

I guess I really needed sleep yesterday.

Awesome work @tmeasday @shilman

[IanVS]

This is great, and means I think we can get rid of our custom utility that converted absolute paths back into "bare" imports.

[tmeasday]

This is great, and means I think we can get rid of our custom utility that converted absolute paths back into "bare" imports.

As discussed, hold your horses on this, absolute paths are coming back!

If I'm understanding correctly, this doesn't return the resolved path, it checks if the resolved path is truthy, and then returns the original path, or undefined.

That's correct. But I am going to tweak it to absolutize the path (but not resolve the export).

The rationale is that because addons can reference each other by name (e.g. in package-a/preset.js - "addons": ['package-b']), we need to resolve package-b relative to package-a, as we may not be allowed to import package-b as we don't have a direct dependency on it (e.g. in yarn pnp).

[ndelangen]

This is weird... so sb-bench IS running on localhost, but not reachable (assumed)?

[ndelangen]

Looks like when I fixed it and merged it to next next was already broken.
https://app.circleci.com/pipelines/github/storybookjs/storybook/28750/workflows/64bcd849-f71b-43e0-b213-e3c1ed01b441/jobs/407053

[ndelangen]

argh, seems it's not consistent @tmeasday nothing changed between the CI being green and showing the error again.

What a pain.

[IanVS]

Seems like the error is not related to this PR though, right? Any reason not to merge it?

Edit: I guess the chromatic snapshots are still failing, which as I understand was the reason for this PR.