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

fix: component exports #166

Merged
merged 6 commits into from Mar 19, 2024
Merged

Conversation

adamdehaven
Copy link
Contributor

@adamdehaven adamdehaven commented Mar 16, 2024

When the MDCRenderer component export was removed in 9dc5cc3 it actually broke the package.json > exports ability to resolve the component import.

The change in this PR allows for the following import:

import MDCRenderer from '@nuxtjs/mdc/runtime/components/MDCRenderer.vue'

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?

@adamdehaven
Copy link
Contributor Author

@farnabaz I can spin up a reproduction if you need to test

package.json Outdated
@@ -24,6 +24,14 @@
"types": "./dist/runtime/index.d.ts",
"import": "./dist/runtime/index.mjs"
},
"./runtime/components/MDCRenderer": {
Copy link
Collaborator

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.

Copy link
Contributor Author

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?

Copy link
Contributor Author

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?

Copy link
Collaborator

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 👍

Copy link
Contributor Author

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.

declaration file error

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?

Copy link
Collaborator

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

@farnabaz
Copy link
Collaborator

Thanks, @adamdehaven
I just add a comment. And feel free to drop PR for test 👍

README.md Outdated Show resolved Hide resolved
@farnabaz farnabaz merged commit eacac48 into nuxt-modules:main Mar 19, 2024
1 check passed
@adamdehaven adamdehaven deleted the fix/component-exports branch March 19, 2024 14:58
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

2 participants