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

chore: compiler-dom as optionalDependencies #2107 #2177

Closed
wants to merge 1 commit into from

Conversation

stefpb
Copy link

@stefpb stefpb commented Aug 31, 2023

Otherwise you get with yarn berry an error
@vue/test-utils tried to access @vue/compiler-dom (a peer dependency) but it isn't provided by your application; this makes the require call ambiguous and unsound.

May cause #1958 again. May fixes #2107.

Otherwise you get with yarn berry an error
@vue/test-utils tried to access @vue/compiler-dom (a peer dependency)
but it isn't provided by your application; this makes the require call
ambiguous and unsound.

May cause vuejs#1958 again. May fixes vuejs#2107.
@netlify
Copy link

netlify bot commented Aug 31, 2023

Deploy Preview for vue-test-utils-docs ready!

Name Link
🔨 Latest commit ebeb505
🔍 Latest deploy log https://app.netlify.com/sites/vue-test-utils-docs/deploys/64f104f7524bde00072b7528
😎 Deploy Preview https://deploy-preview-2177--vue-test-utils-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

package.json Show resolved Hide resolved
Copy link
Collaborator

@freakzlike freakzlike left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot tell if optionalDependencies is the correct one. Might be good to release a beta version and try it out?

@thebanjomatic
Copy link

thebanjomatic commented Sep 25, 2023

Optional dependencies and optional peer-dependencies are two completely different things.

A package listed in optionalDependencies will not fail the install if package fails its post install hooks, etc. This is most frequently used for packages that have platform dependencies due to native modules (this package only works on linux). I don't believe this applies in the case of @vue/compiler-dom, so this PR is essentially the same as just moving it to dependencies.

What appears to be happening here is just that:
@vue/compiler-dom is being accessed here, but appears to not be listed as a peer dependency.

Note that as currently implemented, both "@vue/compiler-dom" and "@vue/server-renderer" aren't optional as the code fails to run without these packages installed:

  ● Test suite failed to run
                                                                                                                                                                                           
    Cannot find module '@vue/server-renderer' from 'node_modules/vue3-test-utils/dist/vue-test-utils.cjs.js'

    Require stack:
      node_modules/vue3-test-utils/dist/vue-test-utils.cjs.js
      ...

Looking at the compiled output, this is because there are imports at the top of the file that still exist in an unchecked fashion regardless of whether not renderToString or processSlot are called.

I believe the correct fix for this PR would be to make both @vue/compiler-dom and @vue/server-renderer optional peer dependencies (like @vue/server-renderer is currently implemented) and then to fix the imports so that we don't import/require from the packages unless you need to use them.

@thebanjomatic
Copy link

I have put up an alternate PR for this: #2197 which at least for sure fixes the @vue/server-renderer issue I was having.

@vue/compiler-dom on the other hand is a little weirder as this package seems to be relying on the fact that it will be a dependency of vue and @vue/compiler-sfc and that it will get installed and hoisted to a level where this package will be able to see and access it. Generally speaking, that is unreliable, and marking it as a peer dependency would allow people to explicitly install it to make this work, but if it happens to be hoisted to the right places, the above PR would allow things to continue to just work.

That said, it's not great from a developer experience perspective, and I believe that is one of the reasons why @vue/compiler-sfc is re-exported from vue through vue/compiler-sfc as it removes the need to have an explicit dependency. Would it be possible to get compiler-sfc to re-export the compile function so that it can be accessed from vue/compiler-sfc instead?

@lmiller1990
Copy link
Member

In favor of #2197

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug with yarn berry: @vue/test-utils tried to access @vue/compiler-dom
5 participants