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.

Sign up for GitHub

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

Jump to bottom

fix: component exports #166

Merged
merged 6 commits into from
Mar 19, 2024
Merged

fix: component exports #166

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d043cd4
fix: component exports
adamdehaven Mar 16, 2024
49bd123
docs: update import
adamdehaven Mar 16, 2024
2fe0cc7
chore: remove pack
adamdehaven Mar 16, 2024
0829928
fix: add dist
adamdehaven Mar 16, 2024
d3108ec
chore: adjust component exports
adamdehaven Mar 19, 2024
b6f0e11
Update README.md
farnabaz Mar 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ Now import the `useMarkdownParser` composable we just created along with an expo
<script setup lang="ts">
import { onBeforeMount, ref, watch } from 'vue'
// Import package exports
import { MDCRenderer } from '@nuxtjs/mdc/runtime/components/MDCRenderer'
import MDCRenderer from '@nuxtjs/mdc/runtime/components/MDCRenderer'
farnabaz marked this conversation as resolved.
Show resolved Hide resolved
import type { MDCParserResult } from '@nuxtjs/mdc/runtime/types/index'
import { useMarkdownParser } from './composables/useMarkdownParser';

Expand Down
8 changes: 8 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

"types": "./dist/runtime/components/MDCRenderer.vue.d.ts",
"import": "./dist/runtime/components/MDCRenderer.vue"
},
"./dist/runtime/components/MDCRenderer": {
"types": "./dist/runtime/components/MDCRenderer.vue.d.ts",
"import": "./dist/runtime/components/MDCRenderer.vue"
},
"./runtime/*": {
"types": "./dist/runtime/*.d.ts",
"import": "./dist/runtime/*.mjs"
Expand Down