Skip to content
This repository has been archived by the owner on Dec 19, 2023. It is now read-only.

chore(deps): update test packages #290

Closed
wants to merge 18 commits into from
Closed

chore(deps): update test packages #290

wants to merge 18 commits into from

Conversation

renovate[bot]
Copy link
Contributor

@renovate renovate bot commented Aug 2, 2023 •
edited
Loading

Mend Renovate

This PR contains the following updates:

Package Change Age Adoption Passing Confidence
@vitest/coverage-v8 (source) 0.33.0 -> 0.34.1 age adoption passing confidence
@vitest/ui (source) ^0.33.0 -> ^0.34.0 age adoption passing confidence
happy-dom 10.5.2 -> 10.8.0 age adoption passing confidence
vitest 0.33.0 -> 0.34.1 age adoption passing confidence
vitest ^0.24.5 || ^0.26.0 || ^0.27.0 || ^0.28.0 || ^0.29.0 || ^0.30.0 || ^0.33.0 -> ^0.24.5 || ^0.26.0 || ^0.27.0 || ^0.28.0 || ^0.29.0 || ^0.30.0 || ^0.33.0 || ^0.34.0 age adoption passing confidence

Release Notes

vitest-dev/vitest (@​vitest/coverage-v8)

v0.34.1

Compare Source

   🐞 Bug Fixes
    View changes on GitHub

v0.34.0

Compare Source

   🚨 Breaking Changes
   🚀 Features
   🐞 Bug Fixes
   🏎 Performance
  • Deprecate deps.registerNodeLoader  -  by @​sheremet-va (7f45b)
    • This option was introduced to support aliasing inside external packages. Please, use deps.optimizer.web instead. If you test Node.js applications, consider adding external packages to server.deps.inline.
    View changes on GitHub
capricorn86/happy-dom (happy-dom)

v10.8.0

Compare Source

👷‍♂️ Patch fixes
  • Fixes bug related to item being null in HTMLButtonElement.attributes.removeNamedItem(). (#​1010)

Thank you @​maxmilton for your contribution!

v10.7.0

Compare Source

🎨 Features
  • Add support for Element.scrollHeight. The property will just return "0" for now. (#​991)

Thank you @​lukaselmer for your contribution!

v10.6.3

Compare Source

👷‍♂️ Patch fixes
  • Adds missing named CSS colors to CSSStyleDeclaration. (#​1004)

Thank you @​AlexisTessier for your contribution!

v10.6.2

Compare Source

v10.6.1

Compare Source

👷‍♂️ Patch fixes
  • Fixes issue where only labels associated using ID where found in HTMLButtonElement.labels, HTMLInputElement.labels, HTMLSelectElement.labels and HTMLTextAreaElement.labels. It will now also return labels associated by being a parent. (#​988)

Thank you @​joshkel for your contribution!

v10.6.0

Compare Source

🎨 Features
  • Improves support for support for HTMLLabelElement.control. It will now find child control elements that are deeper than one level. (#​530)

Thank you @​joshkel for your contribution!

v10.5.3

Compare Source

👷‍♂️ Patch fixes
  • Improves support for NamedNodeMap, which is used by the Element.attributes property. It will now reflect any changes done to it on the Element itself. (#​728)

Configuration

📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 Automerge: Enabled.

♻ Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

👻 Immortal: This PR will be recreated if closed unmerged. Get config help if that's undesired.

  • If you want to rebase/retry this PR, check this box

This PR has been generated by Mend Renovate. View repository job log here.

@socket-security
Copy link

socket-security bot commented Aug 2, 2023 •
edited
Loading

Updated dependencies detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives Size Publisher
vitest 0.33.0...0.34.6 environment +8/-8 24.8 MB oreanno
vite 4.4.11...4.4.9 None +1/-1 5.85 MB vitebot

@renovate renovate bot changed the title chore(deps): update test packages to v0.34.1 chore(deps): update test packages Aug 2, 2023
@renovate renovate bot force-pushed the renovate/test-packages branch 7 times, most recently from 36805e9 to d8ff8a0 Compare August 4, 2023 01:46
@oskarols oskarols mentioned this pull request Aug 4, 2023
Closed
@renovate renovate bot force-pushed the renovate/test-packages branch 10 times, most recently from 58b046d to a489652 Compare August 7, 2023 13:25
@renovate
Copy link
Contributor Author

renovate bot commented Aug 7, 2023

Edited/Blocked Notification

Renovate will not automatically rebase this PR, because it does not recognize the last commit author and assumes somebody else may have edited the PR.

You can manually request rebase by checking the rebase/retry box above.

⚠ Warning: custom changes will be lost.

@danielroe
Copy link
Owner

@sheremet-va Do you have any idea how we can get the vitest-environment-nuxt package to be inlined as previously? We need to because it uses an import alias #app/entry in its environment setup function to import a path alias that is defined in the vitest config it gets from nuxt.

@sheremet-va
Copy link

@sheremet-va Do you have any idea how we can get the vitest-environment-nuxt package to be inlined as previously? We need to because it uses an import alias #app/entry in its environment setup function to import a path alias that is defined in the vitest config it gets from nuxt.

deps.inline is just moved to server.deps.inline, and it will not be removed on release.

@danielroe
Copy link
Owner

danielroe commented Aug 14, 2023 •
edited
Loading

Strangely, even though we've made that change in this PR, it still fails to run. (In other words, it's not behaving the same as deps.inline did before.)

@sheremet-va
Copy link

sheremet-va commented Aug 14, 2023 •
edited
Loading

Strangely, even though we've made that change in this PR, it still fails to run. (In other words, it's not behaving the same as deps.inline did before.)

What I'm seeing in the PR is that Vite resolves vitest-environment-nuxt to require condition. Why it does this I'm not sure (because no ssr flag anymore?). inline never affected how imports are resolved.

Anyway, publishing CJS entry seems pointless when you import from vitest since we don't provide any CJS support for the public API.

@danielroe
Copy link
Owner

Checking when removing CJS builds, the issue is that local-pkg does not have support for pure ESM packages and uses CJS resolution strategy.

But even when patching vitest, it doesn't solve the issue. We still end up with the following error:

TypeError: Package import specifier "#app/entry" is not defined in package /Users/daniel/code/vitest-environment-nuxt/packages/vitest-environment-nuxt/package.json imported from /Users/daniel/code/vitest-environment-nuxt/packages/vitest-environment-nuxt/dist/index.mjs
 ❯ new NodeError node:internal/errors:387:5
 ❯ throwImportNotDefined node:internal/modules/esm/resolve:354:9
 ❯ packageImportsResolve node:internal/modules/esm/resolve:737:3
 ❯ moduleResolve node:internal/modules/esm/resolve:895:21
 ❯ defaultResolve node:internal/modules/esm/resolve:1115:11
 ❯ nextResolve node:internal/modules/esm/loader:163:28
 ❯ ESMLoader.resolve node:internal/modules/esm/loader:841:30
 ❯ ESMLoader.getModuleJob node:internal/modules/esm/loader:424:18
 ❯ ESMLoader.import node:internal/modules/esm/loader:525:22
 ❯ importModuleDynamically node:internal/modules/esm/translators:110:35

Note that this is a regression in this version of vitest - it's not replicable with the exact same code in 0.33.

@danielroe danielroe mentioned this pull request Aug 14, 2023
Merged
6 tasks
@sheremet-va
Copy link

An update to environments is released in 0.34.4

@danielroe
Copy link
Owner

@sheremet-va We now have a new issue with vite-node - it looks at first glance like an issue with mocking CJS utils.

Run logs

⎯⎯⎯⎯⎯⎯ Unhandled Error ⎯⎯⎯⎯⎯⎯⎯
Error: Cannot find module './browser.js'
Require stack:
- /home/runner/work/nuxt-vitest/nuxt-vitest/playground/debug
 ❯ Module._resolveFilename node:internal/modules/cjs/loader:1077:15
 ❯ Module._load node:internal/modules/cjs/loader:922:27
 ❯ Module.require node:internal/modules/cjs/loader:1143:19
 ❯ require node:internal/modules/cjs/helpers:121:18
 ❯ debug:7:19
 ❯ ViteNodeRunner.runModule ../node_modules/.pnpm/vite-node@0.34.5_@types+node@20.5.7/node_modules/vite-node/dist/client.mjs:342:11
 ❯ ViteNodeRunner.directRequest ../node_modules/.pnpm/vite-node@0.34.5_@types+node@20.5.7/node_modules/vite-node/dist/client.mjs:326:16
 ❯ ViteNodeRunner.cachedRequest ../node_modules/.pnpm/vite-node@0.34.5_@types+node@20.5.7/node_modules/vite-node/dist/client.mjs:189:14
 ❯ ViteNodeRunner.dependencyRequest ../node_modules/.pnpm/vite-node@0.34.5_@types+node@20.5.7/node_modules/vite-node/dist/client.mjs:223:12
 ❯ async /home/runner/work/nuxt-vitest/nuxt-vitest/playground/vite-node/client:6:31

Checking the vite node context, we have this:

{
    __filename: '/Users/daniel/code/nuxt-vitest/playground/debug',
    __dirname: '/Users/daniel/code/nuxt-vitest/playground'
}

These seem like the wrong results for the transform request, but I could be wrong.

@sheremet-va sheremet-va mentioned this pull request Sep 28, 2023
Merged
6 tasks
@sheremet-va
Copy link

sheremet-va commented Sep 28, 2023 •
edited
Loading

Should be fixed by: vitest-dev/vitest#4196

I am a bit surprised it works like this, looks like a potential performance optimization.

@danielroe
Copy link
Owner

@sheremet-va I really appreciate all your help on this ❤️

@sheremet-va
Copy link

Fix is released in 0.34.6

@antfu
Copy link
Collaborator

antfu commented Oct 3, 2023

The error now is caused by multiple instances of Vue. Tried with VITE_NODE_DEBUG_DUMP=1

In nuxt/dist/app/entry.js is importing vue and nuxt/dist/app/nuxt.js, while nuxt/dist/app/nuxt.js is importing /node_modules/.vitest/deps/vue.js?v=2c47765e.

I am able to fix most of the tests with (still some failing with other errors):

test: {
    deps: {
      optimizer: {
        web: {
          enabled: false
        }
      }
    },
}

I am a bit rusted with Vitest, I guess @sheremet-va might have a better context?

@antfu
Copy link
Collaborator

antfu commented Oct 3, 2023

I also found that, in defineVitestConfig we use mergeConfig from Vite, which does not merge but overrides the test object. Meaning that if a user provides a custom test option, the entire default config from Nuxt will be ignored. Fixed

@sheremet-va
Copy link

sheremet-va commented Oct 3, 2023 •
edited
Loading

I am a bit rusted with Vitest, I guess @sheremet-va might have a better context?

Even though the environment file is loaded in the same scope as your tests, it is now always processed with ssr flag which means it can resolve to a different file (so, environment imports vue from vue and tests import vue from .deps/vue). This is probably the cause of the issues here.

I would recommend rewriting the environment to be environment agnostic (don't rely on imports because they are environment-specific), to be honest. To support VM (which is faster and might be the direction Vitest takes in the future), you would need to rewrite it with a similar notion in mind.

@oskarols
Copy link
Contributor

@danielroe @antfu It's a bit hard to grasp the scope of changes necessary here, is there anything we could do to help this along? 😄

This was referenced Nov 20, 2023
Merged
Merged
@danielroe danielroe mentioned this pull request Dec 1, 2023
Merged
@danielroe danielroe closed this Dec 19, 2023
@danielroe danielroe deleted the renovate/test-packages branch December 19, 2023 16:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
chore dependencies nuxt-vitest
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@danielroe @sheremet-va @antfu @oskarols