Open mcous opened 3 months ago
It's an issue others have mentioned before, but ultimately, this is a completely valid configuration we have here and it's other tools that are causing the issue.
Using the exports.browser
condition in Node is completely misusing it and applying Node's file extension semantics to it (a module not meant to be used in Node at all) is incorrect. Vitest (as well as Jest w/ JSDOM) should not be using a module meant for use in the browser anywhere but a browser. Our browser build adheres only to the conditions of the intended runtime environment, not Node or any other runtime.
Additionally, altering package.json#exports
is a breaking change and one that cannot occur outside of a major, so we can't do much here until v11 (though I still think we shouldn't really do anything here; it's a bug/misuse in other tools).
Additional references: https://github.com/preactjs/jest-preset-preact/pull/13#issuecomment-1404316519, https://github.com/preactjs/preact/pull/3634
Thanks for the detailed response and additional tickets! Apologies that my cursory search didn't turn them up. I definitely see (and agree with much of) where you're coming from here.
Out of curiosity, what are the differences between the preact.module.js
and preact.mjs
? It doesn't look to me like there are any, based on a quick diff.
It's an issue others have mentioned before, but ultimately, this is a completely valid configuration we have here and it's other tools that are causing the issue.
I might quibble with "completely valid configuration" here: package.json
is consumed by Node / bundlers, not browsers directly. This package.json
says: "here is a browser
export, and it is CJS, according to the rules of exports
conditions and the type
field." Setting aside Vitest and the act of importing the browser bundle in Node, this seems like an incorrect configuration.
For example, I think this would be a more accurate (completely untested) export that says, "here is a browser
export, and it is in ESM format:"
{
"exports": {
".": {
"types": "./src/index.d.ts",
"browser": {
"import": "./dist/preact.module.js",
},
"umd": "./dist/preact.umd.js",
"import": "./dist/preact.mjs",
"require": "./dist/preact.js"
},
}
}
Vitest (as well as Jest w/ JSDOM) should not be using a module meant for use in the browser anywhere but a browser. Our browser build adheres only to the conditions of the intended runtime environment.
I agree with most of this, and I certainly don't think Preact or any module should, when providing a browser
export, expect it to run anywhere other than the browser. Vitest, to its credit, will prioritize Node exports for packages. The user must actively opt-in to resolving browser
exports during tests.
But I think a user should be able to make that choice! For a lot of us, the upsides of running browser-intended code against a fake DOM outweigh the downsides of using an inaccurate environment. So, while no module should go out of their way to support this sort of runtime, I do think a module should accurately report its exports so that a user can choose to do this and take on whatever risk they feel is appropriate.
Additionally, altering package.json#exports is a breaking change and one that cannot occur outside of a major, so we can't do much here until v11 (though I still think we shouldn't really do anything here; it's a bug/misuse in other tools).
Yeah, definitely hear this. I saw in one of the threads you linked that this won't be an issue in v11
, do you know if that's true? I saw that the v11
branch has no browser
export conditions, but I don't know if that's up to date.
I wonder, but do not know, if something like my example above would be non-breaking. I'm not sure what tools are currently relying on the browser
export nor what tools y'all expect to be relying on it
Out of curiosity, what are the differences between the preact.module.js and preact.mjs?
None at the moment, and I think the latter is even generated from the former. We could make use of this but haven't (not sure if we will either).
I might quibble with "completely valid configuration" here:
package.json
is consumed by Node / bundlers, not browsers directly.
package.json
is metadata for JS packages; this could be used by runtimes, bundlers, or CDNs. The last two don't need to adhere to Node semantics.
Node's docs pretty specifically state it does not govern any keys/conditions that Node itself does not consume which was a key design decision from the start. Additional keys are, per the spec, completely valid and can have their own semantics. At the moment, we do happen to make use of this.
and it is CJS, according to the rules of
exports
conditions and thetype
field.
browser
isn't beholden to the type
field (or shouldn't be, rather) as it isn't ever consumed by Node. File extension semantics simply don't apply to it, same as module
(it is always ESM).
I do think a module should accurately report its exports so that a user can choose to do this and take on whatever risk they feel is appropriate.
I mean, we do, for the intended environment. If you want to use something in an env it's not meant to run in, there be dragons and all that. .js
ESM is totally valid in the browser.
I saw in one of the threads you linked that this won't be an issue in v11, do you know if that's true? I saw that the v11 branch has no browser export conditions, but I don't know if that's up to date.
Can't say for sure; v11 has no planned release date at this time (we've gotten a lot more use out of v10 than we originally expected, delaying the need for a new major) and so we might revise our export conditions further before release. The v11 is indeed "up to date", but early beta-ish and things may change.
if something like my example above would be non-breaking.
It might, but with how fragile exports
is in general, I wouldn't really trust it. Some env is (sadly) likely to fall over with any change. I think the general community consensus is still to avoid touching between majors if at all possible which is why I think our position has to be "can't fix" for now.
Describe the bug
Hello! I'm coming here from testing-library/svelte-testing-library#381, where a user reported interop issues when using
preact
together with oursvelteTesting
Vite plugin. Our plugin makes one significant change to the Vite config: it addsbrowser
toresolve.conditions
to ensure Svelte's browser code is imported.This also causes Preact's
browser
export -./dist/preact.module.js
- to be used:The bug is:
preact.module.js
is ESMpackage.json
does not specifictype
, so"type": "commonjs"
is impliedpreact.module.js
ends in.js
rather than.mjs
, it is treated as CommonJSAt worst, this produces a fairly inscrutable error:
With enough fiddling, I can get Vite to complain in this manner:
If I go in and manually rename files to
.module.mjs
and update thepackage.json
, everything works happily.To Reproduce
I can put together a minimal reproduction if required - just let me know. There is also this repro from the original reporter: https://github.com/molily/vitest-preact-svelte
Expected behavior
I should be able to load the
browser
copy of Preact in Vitest without issue