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: add proper dependencies #1950

Merged
merged 1 commit into from Jan 27, 2023

Conversation

cexbrayat
Copy link
Member

We were only declaring devDependencies, so this refactors the package.json to use dependencies when it makes sense. @vue/runtime-core was also used in some imports, where we could use vue directly, so this commit changes this as well.

@netlify
Copy link

netlify bot commented Jan 26, 2023

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

Name Link
🔨 Latest commit cfdbe83
🔍 Latest deploy log https://app.netlify.com/sites/vue-test-utils-docs/deploys/63d28474d6550200094cd942
😎 Deploy Preview https://deploy-preview-1950--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 settings.

@lmiller1990
Copy link
Member

Does this make sense? What if people have a different version of Vue 3 to the one we specified? I am not sure if Test Utils should have a hard dependency on a specific version of Vue 3. Vue Router, for example, does not: https://github.com/vuejs/vue-router/blob/dev/package.json

@cexbrayat
Copy link
Member Author

I'm not sure about what should be the best practice TBH. @Jinjiang pointed that out on Discord, but maybe this should stay as dev deps and all 3 deps should be specified in peer deps? Or maybe we specify a range for the deps?

@freakzlike
Copy link
Collaborator

peerDependencies might be the correct on for @vue/compiler-dom and vue. I would still set js-beautify as dependency and pin to 1.14.6 until #1834 is fixed

@Jinjiang
Copy link
Member

I would agree with @freakzlike if we just don't want to lock the version for the users, then peer dependencies would work.

We were only declaring `devDependencies`, so this refactors the `package.json` to use `dependencies` and `peerDependencies` when it makes sense.
`@vue/runtime-core` was also used in some imports, where we could use `vue` directly, so this commit changes this as well.
@cexbrayat
Copy link
Member Author

I updated the PR with:

  • js-beautify as a (set) dependency
  • compiler-dom as a peer dep

Please take another look @lmiller1990 @freakzlike @Jinjiang when you have some time and let me know what you think.

(I wonder if we should have ^3.0.1 for the Vue peer deps, I'm not sure this is correct, as we probably depend on stuff introduced in way more recent versions... but this can be changed in another PR)

@lmiller1990 lmiller1990 merged commit bd3bab8 into vuejs:main Jan 27, 2023
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.

None yet

4 participants