-
Notifications
You must be signed in to change notification settings - Fork 667
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
build: add esm bundle #1990
build: add esm bundle #1990
Conversation
❌ Deploy Preview for vue-test-utils-v1 failed.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, can you also add the exports
field like so?
https://github.com/vuejs/test-utils/blob/main/package.json#L9-L15
39fec1a
to
838bb9a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, will do a release soon :)
"exports": { | ||
".": { | ||
"import": { | ||
"node": "./dist/vue-test-utils.js", | ||
"require": "./dist/vue-test-utils.js", | ||
"default": "./dist/vue-test-utils.esm.js" | ||
} | ||
} | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this added as-is, i.e. why with the import
nesting? That breaks importing @vue/test-utils
v1.3.1 in jest (non esm) for me with:
Cannot find module '@vue/test-utils' from '...'
> 1 | import { createLocalVue } from '@vue/test-utils'
when I remove the import
like this:
"exports": {
".": {
"node": "./dist/vue-test-utils.js",
"require": "./dist/vue-test-utils.js",
"default": "./dist/vue-test-utils.esm.js"
}
},
then it works again.
The comment that suggested to add the exports
key was referencing this, i.e. the vue3 version of test-utils:
https://github.com/vuejs/test-utils/blob/main/package.json#L9-L15
There is no nested import
there.
Maybe the following would be best?
"exports": {
".": {
"import": "./dist/vue-test-utils.esm.js",
"require": "./dist/vue-test-utils.js",
"default": "./dist/vue-test-utils.js"
}
},
That would be the most similar to what is done in vue/test-utils right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uh oh, did not realize this broke things. I'll patch this right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
https://github.com/vuejs/vue-test-utils/releases/tag/v1.3.2
Gave it a quick test, seems okay, LMK if you run into any problems.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks that fixed it!
What kind of change does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
If yes, please describe the impact and migration path for existing applications:
The PR fulfills these requirements:
dev
branch.fix #xxx[,#xxx]
, where "xxx" is the issue number)If adding a new feature, the PR's description includes:
Other information:
Provides an ESM bundle.