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

feat!: use vue-component-meta #34

Merged
merged 6 commits into from Aug 29, 2022

Conversation

stafyniaksacha
Copy link
Collaborator

@stafyniaksacha stafyniaksacha commented Aug 1, 2022

⚠️ This introduces breaking change in schemas (more details here)

This is the first implementation of vue-component-meta to extract props/events/slots/exposed using typescript vue language service parser from volar
- replaces #23

It can force usage of typescript in javascript components (thanks that vue3 is written in typescript) so detection work, but it needs a tsconfig.json in the root of the project. (can be achieved within vue-component-meta)

#nuxt-component-meta virtual module is exposed, so we can use meta inside the project without using a $fetch (useful to create components documentation)
- replaces #32

  • tests (awaiting feedback before updating them)

@Atinux Atinux requested a review from farnabaz August 1, 2022 18:00
@farnabaz
Copy link
Collaborator

farnabaz commented Aug 2, 2022

Generally LGTM 👍 , but there are some issues we should deal with:

  • Default values of props are missing
  • In normal components(TestComponent), all props are marked as required

Also, I would love to have @kevinmarrec review, about the schema and possible breaking changes for the editor.

@stafyniaksacha
Copy link
Collaborator Author

  • Default values of props are missing
  • In normal components(TestComponent), all props are marked as required

Should be resolved with vuejs/language-tools#1665

@Tahul
Copy link
Contributor

Tahul commented Aug 10, 2022

@kevinmarrec @farnabaz ; could we state on this PR quickly?

I would love to see this happen to iterate on Studio Elements components documentation we're building.

@farnabaz
Copy link
Collaborator

Everything looks good to me, The only remaining thing is the tests.
@stafyniaksacha Do you mind updating tests?

@stafyniaksacha
Copy link
Collaborator Author

Everything looks good to me, The only remaining thing is the tests. @stafyniaksacha Do you mind updating tests?

Yep, I'm on it.

I've found that the library is not able to load components from node_modules (this affects autoloaded components library)
I don't know if this is a required feature for now, but I think this will be fixed upstream on vue-component-meta

@Tahul
Copy link
Contributor

Tahul commented Aug 27, 2022

Hey @stafyniaksacha ; noticed some changes 😄

Is this PR ready for review again?

I'm willing to have this ASAP for usage in multiple Nuxt packages, that work is amazing!

@stafyniaksacha
Copy link
Collaborator Author

stafyniaksacha commented Aug 27, 2022

Hey @Tahul ! Yes pretty much ready, waiting on next release of vue-component-meta, so we can get rid of tsconfig.json that is required now.
https://github.com/johnsoncodehk/volar/blob/master/packages/vue-component-meta/tests/index.spec.ts#L684-L693

This is why the tests are failing, we can not create one in tests/fixtures/base because @nuxt/test-utils will create a random folder under .nuxt

{
  "extends": "./.nuxt/random-xxx/tsconfig.json"
}

https://github.com/nuxt/framework/blob/main/packages/test-utils/src/nuxt.ts#L42

@stafyniaksacha
Copy link
Collaborator Author

stafyniaksacha commented Aug 27, 2022

Hey @Tahul @farnabaz
That's should be ready ;)

@stafyniaksacha stafyniaksacha changed the title feat: use vue-component-meta feat!: use vue-component-meta Aug 27, 2022
Copy link
Collaborator

@farnabaz farnabaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks ❤️

@farnabaz farnabaz merged commit f17413d into nuxtlabs:main Aug 29, 2022
@stafyniaksacha stafyniaksacha deleted the feat/vue-component-meta branch August 29, 2022 10:50
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

3 participants