-
Notifications
You must be signed in to change notification settings - Fork 39
fix: component exports #166
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
fix: component exports #166
Conversation
|
@farnabaz I can spin up a reproduction if you need to test |
package.json
Outdated
| "types": "./dist/runtime/index.d.ts", | ||
| "import": "./dist/runtime/index.mjs" | ||
| }, | ||
| "./runtime/components/MDCRenderer": { |
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.
Let's add components directory in exports and not just MDCRenderer.
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.
I don't believe this is possible with the current state of the repo and exported types.
There is no generated runtime/components/index.mjs so we can't import directly from there. Also, the MDC.vue component and none of the runtime/components/prose/*.vue files are generating types in the build.
If we created a runtime/components/index.ts file that exported all of the components (a barrel file) it would then trigger the proper exports, but could have an impact on tree-shaking; however, the other components like MDC.vue utilize Nuxt-specific imports, meaning it would actually break using in a non-Nuxt project.
Since only the MDCRenderer.vue component was "cleaned" of Nuxt imports, exporting anything else as-is would break 😬
Good with this for now, or any ideas?
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.
@farnabaz here's the updated info.
As for the tests, if I was going to write something I'd likely just want to use vitest along with @vue/test-utils so that I can ensure nothing from the Nuxt app is required. I'd need to add that package as a devDependency; any issues there?
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 not just use "./runtime/components/*": "./dist/runtime/components/*", in package exports? Then we can import like import MDCRenderer from '@nuxtjs/mdc/runtime/components/MDCRenderer.vue';
Vitest and Vue test-utils seems fine for me 👍
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.
@farnabaz I adjusted the exports in d3108ec to this:
"./runtime/components/*": {
"types": "./dist/runtime/components/*.d.ts",
"import": "./dist/runtime/components/*"
},
"./dist/runtime/components/*": {
"types": "./dist/runtime/components/*.d.ts",
"import": "./dist/runtime/components/*"
}For the types property, I had to add *.d.ts in order to properly resolve the component types.
Technically this doesn't allow for importing types from the nested /runtime/components/prose/* components, but the build command doesn't generate declaration files for these components anyway, so I think it's fine?
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.
LGTM 👍
We can update those components later to generate ts file
|
Thanks, @adamdehaven |

When the
MDCRenderercomponent export was removed in 9dc5cc3 it actually broke thepackage.json > exportsability to resolve the component import.The change in this PR allows for the following import:
I updated the docs, but also wondering if it would make sense to have test coverage for a non-Nuxt implementation in a separate PR?