Skip to content
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.

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

Bug: ReferenceError: Vue is not defined - Missing exports declaration ? #2462

Open
sronveaux opened this issue Jun 27, 2024 · 8 comments
Open
Labels
bug Something isn't working

Comments

@sronveaux
Copy link

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 the package.json of VTU like this:

image

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 using Webpack then executes the result inside a browser directly and not using node and JSDom as lots of "modern" test runners do. The target parameter inside webpack config file is set to default (so browserslist or web).

Thanks in advance ;-)

@sronveaux sronveaux added the bug Something isn't working label Jun 27, 2024
@lmiller1990
Copy link
Member

lmiller1990 commented Jul 21, 2024

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.

"module": "dist/vue-test-utils.esm-bundler.mjs",

@sronveaux
Copy link
Author

sronveaux commented Jul 23, 2024

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...

@lmiller1990
Copy link
Member

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.

@sronveaux
Copy link
Author

sronveaux commented Jul 24, 2024

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...

@sronveaux
Copy link
Author

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.

@lmiller1990
Copy link
Member

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.

@sronveaux
Copy link
Author

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...

@lmiller1990
Copy link
Member

Second options seems the best imo - what to make a PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants