Skip to content

Export test-utils from compat #4783

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
rschristian merged 3 commits into from
Jun 4, 2025
Merged

Export test-utils from compat #4783

rschristian merged 3 commits into from
Jun 4, 2025

Conversation

@rawrmonstar
Copy link
Contributor

@rawrmonstar rawrmonstar commented Jun 4, 2025

Hello!

react-dom exports test-utils as react-dom/test-utils (https://github.com/facebook/react/blob/d742611ce40545127032f4e221c78bf9f70eb437/packages/react-dom/package.json#L113C5-L113C39)

since it is very common to resolve react and react-dom to preact/compat the fact that preact/compat/test-utils isn't a valid export can break some things.

I noticed this trying to run storybook preact. Storybook Vite lists 'react-dom/test-utils' as an optimizeDependency candidate. Since this ultimately tries to get resolved as preact/compat/test-utils it breaks.

@github-actions
Copy link

github-actions bot commented Jun 4, 2025 •
edited
Loading

📊 Tachometer Benchmark Results

Summary

A summary of the benchmark results will show here once they finish.

Results

The full results of your benchmarks will show here once they finish.

tachometer-reporter-action v2 for CI

@rschristian
Copy link
Member

rschristian commented Jun 4, 2025 •
edited
Loading

This should already be aliased for you with the Vite preset: https://github.com/preactjs/preset-vite/blob/243a8f0f6c8235e6c01f8f0cce791350dfd0796b/src/index.ts#L273

Not that I'm against this, though I think we need to make a decision what should be aliases and what should be export paths (separate from this PR of course).

@rawrmonstar
Copy link
Contributor Author

rawrmonstar commented Jun 4, 2025

@rschristian it does not seem to work in storybook at least. I suppose it could be considered a bug with storybook, but I personally think preact/compat should have the same exports as react and react-dom

npm create vite@latest
  • preact
  • typescript

cd into project

npm create storybook@latest

then run the project, you will get something like this:

=> Failed to build the preview
Error: Missing "./compat/test-utils" specifier in "preact" package
    at e (file://./node_modules/vite/dist/node/chunks/dep-DBxKXgDP.js:12200:25)
    at n (file://./node_modules/vite/dist/node/chunks/dep-DBxKXgDP.js:12200:631)
    at o (file://./node_modules/vite/dist/node/chunks/dep-DBxKXgDP.js:12200:1293)
    at resolveExportsOrImports (file://./node_modules/vite/dist/node/chunks/dep-DBxKXgDP.js:16217:18)
    at resolveDeepImport (file://./node_modules/vite/dist/node/chunks/dep-DBxKXgDP.js:16230:25)
    at tryNodeResolve (file://./node_modules/vite/dist/node/chunks/dep-DBxKXgDP.js:16060:18)
    at ResolveIdContext.resolveId (file://./node_modules/vite/dist/node/chunks/dep-DBxKXgDP.js:15831:19)
    at EnvironmentPluginContainer.resolveId (file://./node_modules/vite/dist/node/chunks/dep-DBxKXgDP.js:42213:17)
    at ResolveIdContext.resolve (file://./node_modules/vite/dist/node/chunks/dep-DBxKXgDP.js:42420:37)
    at ResolveIdContext.resolveId (file://./node_modules/vite/dist/node/chunks/dep-DBxKXgDP.js:12092:25)

https://github.com/rawrmonstar/vite-preact-storybook-error

rschristian
rschristian reviewed Jun 4, 2025
View reviewed changes
@rschristian
Copy link
Member

rschristian commented Jun 4, 2025

but I personally think preact/compat should have the same exports as react and react-dom

Yeah I think that's fair, those aliasing instructions were written before package.json#exports was spec'd (much less supported in any wide capacity) anyhow.

@rschristian rschristian merged commit c40d78a into preactjs:main Jun 4, 2025
4 checks passed
@rschristian
Copy link
Member

rschristian commented Jun 4, 2025

Cheers!

@rawrmonstar rawrmonstar deleted the export-test-utils-from-compat branch June 4, 2025 01:41
@JReinhold JReinhold mentioned this pull request Jun 10, 2025
Closed
@JReinhold
Copy link

JReinhold commented Jun 10, 2025

@rschristian do you have any idea on when this will be released? Is it a v10 or v11 thing?

@rschristian
Copy link
Member

rschristian commented Jun 10, 2025

It's not a breaking change, it'll go out next time we make a v10 release. No ETA on that though.

@marvinhagemeister marvinhagemeister mentioned this pull request Jun 11, 2025
Merged
@JoviDeCroock
Copy link
Member

JoviDeCroock commented Jun 11, 2025

Live in https://github.com/preactjs/preact/releases/tag/10.26.9

@rschristian rschristian mentioned this pull request Jul 8, 2025
Merged
@JoviDeCroock JoviDeCroock mentioned this pull request Aug 19, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rschristian rschristian rschristian approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@rawrmonstar @rschristian @JReinhold @JoviDeCroock