Closed sronveaux closed 2 weeks ago
We already have the module export, it's on the very first line in your screenshot. I thought the bundler would look there - it certainly should.
What version of webpack are you using? I wonder if an old webpack version does not respect the top level module
.
https://github.com/vuejs/test-utils/blob/844f5088ff5cde60c578b2aa5956d38901132d43/package.json#L8
Hi @lmiller1990 and thanks for your reply, I alredy saw the module export and that's why I kept this line in my screenshot. As you said, clearly, Webpack
seems to 'miss' the top level module
.
I'm using an almost up-to-date stack here, Webpack
is on version 5.91.0
. Latest version is 5.93.0
.
I completely agree with you that Webpack
should look there, but it also clearly seems it doesn't... I'm not that much in bundling technologies but what is sure is that top level module
is not part of NPM
standard but was part of a proposal. It's true that it's commonly used by some bundlers.
I'm aware this looks in a sense like a Webpack
issue, however, I opened the issue here as adding a single line can fix the problem... If this is has no impact on other bundlers (I suppose it shouldn't but once again, I'm not an expert at all with the different bundlers available), perhaps it could be added here ?
Edit: After checking more thoroughly in Webpack
documentation, top level module is only briefly mentioned in the authoring libraries section. However, the package exports section mentions the embedded exports
several time and is used inside their examples.
I wonder if this behaviour is following the one implemented in Node where it is clearly stated that once an exports
key is defined, all other entry points are disabled...
What does your project have for type
in package.json
?
I'm hesitant to merge this until we understand the core issue. This package has millions of downloads for week - it's not really ideal to merge a quick fix for one person's project, until we actually understand what's going on.
Hi @lmiller1990,
Sure, I clearly understand your point of view and wasn't expecting a quick fix. Let's hope we can find out what happens here...
Our project has no type
declared in package.json
which is equivalent to commonjs
as explained in Node
documentation.
Edit: I tried to get as much information as possible by delving inside Webpack directly. I'm now almost sure current behaviour is what I wrote at the end of my comment yesterday !
At first, if you look in official Webpack documentation it is stated that exports
fields defines what can be imported, that it replaces the main
field and that once this field is defined, only what is defined inside can be imported and nothing else. So clearly, if this is followed in practice, the top level module
, even if it's implemented, will never be taken into account as it is shadowed by the exports
.
This seems to be confirmed inside the Webpack concept note about module resolution.
And finally, I tried to delve deeper in source code and found that the package which resolves for Webpack
is in fact a separate package called enhanced-resolve. By running the debugger inside it's code, we can see that it searches for a package.json
file, and if found, it uses the exports
field only where module
is not defined...
For my personal case here, the resolve request is made with the require
, module
, webpack
, production
and browser
conditions. From those, browser
is selected as it is the first in the list defined in @vue/test-utils
package.json
exports
field.
The conditions that Webpack
are however not normal at all here: require
and import
should be both defined as stated in Webpack documentation : import and require are both set independent of referencing syntax. require has always lower priority. however the import
condition is not set when resolving this dependency! browser
consition is there because target
inside Webpack
config is set to web
which is normal.
That's all I've found for now... I don't know how other builders are working internally however so once again, I can't tell if adding this line in exports
could do some harm or not but I'm now pretty confident that a top-level module
will not be taken into account by Webpack
once exports
is defined next to it...
I hope this will be of interest for you to better understand what happens in my case for sure, but I think also for all the people who are using Webpack
with target: 'web'
even though this becomes less and less as the standard ways to go nowadays is using Jest
or Vite
...
Just a little word to say that I just edited my last comment... passing too much time on it made me miss the fact that Webpack
was indeed not behaving as expected !
I'd like to make one or two more tests on my side to see why the import
condition is not set...
Also wanted to say that I'll soon be on summer break, so if I don't reply for a certain time, please don't consider the issue as stalled... I hope to finish my tests beforehand but have some other obligations so I'm not sure I'll be able to do them...
Thanks again for your valuable replies.
No problem, will leave this open. We can merge webpack specific code as long as we are confident it won't break other bundlers, webpack is still the most widely used bundler. I know webpack often overreaches and does some weird things, mainly because it was doing that before any of these standards really existed.
Let me know if you need any more info or if you are ready for a review - if we could just try the patch against some other popular tool chains / boilerplates, that'd be fine.
Hi @lmiller1990 and thanks for your understanding and time on this issue.
I couldn't resist to make all the tests I wanted before my leave... here's what I've found and some other proposals that could perhaps better suit depending on your experience with all the tools chains out there...
So, with a current version of Webpack
and on my specific case, I saw that the conditions that were set were require
, module
, webpack
, production
and browser
.
I made some tests with "type": "module"
inside package.json
. Unfortunately, this is not an option in my case as I can't manage to make all dependencies compatible with this, but I could see that, depending on the context where a dependency is imported, the set conditions are either the same, either import
, module
, webpack
, production
and browser
.
Doing some tests with a node-related test runner such as Jest
instead of Karma
, I could see that the conditions were require
, module
, webpack
, production
and node
.
To summarize, my personal problem comes from the fact that browser
has higher priority than require
in VTU exports
field. Someone who is using Jest
should not be impacted because browser
condition is replaced by node
. I'm not sure someone with a "type": "module
setting is safe netiher as sometimes require
condition is set, sometimes it's import
...
My first proposal was indeed to include module
inside exports
like this:
"exports": {
".": {
"types": "./dist/index.d.ts",
"node": "./dist/vue-test-utils.cjs.js",
"module": "./dist/vue-test-utils.esm-bundler.mjs",
"import": "./dist/vue-test-utils.esm-bundler.mjs",
"browser": "./dist/vue-test-utils.browser.js",
"require": "./dist/vue-test-utils.cjs.js",
"default": "./dist/vue-test-utils.cjs.js"
},
"./package.json": "./package.json"
},
However, the main reason in the end is that require
has lower priority to browser
as Webpack
chooses the first exports to meet one of the conditions. So this simple switch also does the trick:
"exports": {
".": {
"types": "./dist/index.d.ts",
"node": "./dist/vue-test-utils.cjs.js",
"import": "./dist/vue-test-utils.esm-bundler.mjs",
"require": "./dist/vue-test-utils.cjs.js",
"browser": "./dist/vue-test-utils.browser.js",
"default": "./dist/vue-test-utils.cjs.js"
},
"./package.json": "./package.json"
},
This second proposal could perhaps be safer and would makes more sense in all cases... except if I miss some use cases, I suppose an IIFE is only used in pure browser context without using modern bundling technologies ?
And eventually, what can also be done as a safety net is to nest multiple conditions in the exports
. That's not very clean and nice but perhaps you'd prefer and think it's safer:
"exports": {
".": {
"types": "./dist/index.d.ts",
"node": "./dist/vue-test-utils.cjs.js",
"import": "./dist/vue-test-utils.esm-bundler.mjs",
"webpack": {
"module": "./dist/vue-test-utils.esm-bundler.mjs"
},
"browser": "./dist/vue-test-utils.browser.js",
"require": "./dist/vue-test-utils.cjs.js",
"default": "./dist/vue-test-utils.cjs.js"
},
"./package.json": "./package.json"
},
This last possibility can be mixed with the second one if you prefer:
"exports": {
".": {
"types": "./dist/index.d.ts",
"node": "./dist/vue-test-utils.cjs.js",
"import": "./dist/vue-test-utils.esm-bundler.mjs",
"webpack": {
"require": "./dist/vue-test-utils.cjs.js"
},
"browser": "./dist/vue-test-utils.browser.js",
"require": "./dist/vue-test-utils.cjs.js",
"default": "./dist/vue-test-utils.cjs.js"
},
"./package.json": "./package.json"
},
So after all those tests and thinking about it, I'd personally propose to take the second option (switching require
and browser
), however, as I don't have a lot of experience with other bundlers than Webpack
and certainly don't have all the knowledge of VTU internals as you've gathered throughout all those years, I prefer to let you evaluate if this could be applied and which version you'd prefer... The second proposal is the one which is the cleanest and it really makes sense I think, but perhaps you'd like the "safety net" of the last proposal...
Just take your time to think about it and tell how you feel, I'll happily open a PR when coming back from my break in case you think one is worth considering...
Second options seems the best imo - what to make a PR?
Describe the bug
Hello, I'm kind of reopening #234 here as I'm having some trouble upgrading a software to Vue3 which was running completely fine in Vue 2 with Karma test runner and Vue Test Utils V1.x latest version.
Once everything is updated, unit tests can't run anymore and a
ReferenceError: Vue is not defined
is thrown. I tried to follow what was written in this post and investigate the different possibilities and found that the easiest way around was to simply add a "module" exports inside thepackage.json
of VTU like this:Is there a good reason not to declare the module exports type here ? I'm not an expert at all in bundling so I don't know if there could be some bad side effects... If not, could it be inserted here ? I'm O.K. to create a simple PR with this if needed...
To give more information on the first reply that was given by @lmiller1990 at that time,
Karma
bundles the unit tests, the project source code and its test runner usingWebpack
then executes the result inside a browser directly and not using node and JSDom as lots of "modern" test runners do. Thetarget
parameter inside webpack config file is set to default (sobrowserslist
orweb
).Thanks in advance ;-)