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

Vue upgrade #33167

Merged
merged 8 commits into from May 2, 2024
Merged

Vue upgrade #33167

merged 8 commits into from May 2, 2024

Conversation

bsmth
Copy link
Member

@bsmth bsmth commented Apr 19, 2024

Description

Updating the docs for Vue in parallel while updating the todo vue app.

Motivation

The versions we were suggesting are no longer current when using Vue starter templates. The prose here is based on experience upgrading in the linked PR.
The https://github.com/mdn/todo-vue app also had some security advisories, so it would be good to be using packages without known vulns.

Related issues and pull requests

TODO

  • Add note about versions this tutorial targets
  • explain tree shaking / bundle splitting

@bsmth bsmth requested review from a team as code owners April 19, 2024 09:24
@bsmth bsmth requested review from hamishwillee and pepelsbey and removed request for a team April 19, 2024 09:24
@github-actions github-actions bot added Content:Learn:Client-side Content under “Client-side JavaScript frameworks” (Svelte, React, Angular, Vue) and related subtrees system Infrastructure and configuration for the project size/m 51-500 LoC changed labels Apr 19, 2024
Copy link
Contributor

github-actions bot commented Apr 19, 2024


In the case of `App.vue`, our default export sets the name of the component to `App` and registers the `HelloWorld` component by adding it into the `components` property. When you register a component in this way, you're registering it locally. Locally registered components can only be used inside the components that register them, so you need to import and register them in every component file that uses them. This can be useful for bundle splitting/tree shaking since not every page in your app necessarily needs every component.
In the case of `App.vue`, two components `TheWelcome` and `HelloWorld` are registered by means of imports. When you register a component in this way, you're registering it locally. Locally registered components can only be used inside the components that register them, so you need to import and register them in every component file that uses them. This can be useful for bundle splitting/tree shaking since not every page in your app necessarily needs every component.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know this pre-exists the change, but consider explaining "bundle splitting/tree shaking"

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea, added a todo above

Copy link
Collaborator

@hamishwillee hamishwillee left a comment

Choose a reason for hiding this comment

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

@bsmth I've added some minor editorial comments/questions.

One question is whether we need to provide some versioning information here about the Vue we're targeting? I.e. Vue3/Vite?

What you've written holds together well. I've added approval, but I'm not a Vue expert by any means and I have not run this code. So you can merge if you like or wait for more expert review.

…meworks/vue_getting_started/index.md

Co-authored-by: Hamish Willee <hamishwillee@gmail.com>
@bsmth
Copy link
Member Author

bsmth commented Apr 22, 2024

Thank you!

One question is whether we need to provide some versioning information here about the Vue we're targeting? I.e. Vue3/Vite?

Good idea. Isn't there a note in related framework guides saying this article targets XYZ, I think we can do the same here. I will add to this PR.

So you can merge if you like or wait for more expert review.

Thank you! I revisited this again after doing an initial run though in January, so I think we're in a good place, but if I can get another pair of eyes on the example repo it would be great. I've tagged Vinyl, but I'll see if I can get someone to have a look in case I've missed anything.

…meworks/vue_rendering_lists/index.md

Co-authored-by: Hamish Willee <hamishwillee@gmail.com>
@bsmth
Copy link
Member Author

bsmth commented May 2, 2024

@hamishwillee - the related code has been merged (linked PR above) and I've added the notes as you suggested (in de67367 and 011cd23)! I'm going to merge now so both repos are in sync. Thanks a lot!

@bsmth bsmth merged commit 1acfcd1 into mdn:main May 2, 2024
8 checks passed
@bsmth bsmth deleted the vue-upgrade branch May 2, 2024 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:Learn:Client-side Content under “Client-side JavaScript frameworks” (Svelte, React, Angular, Vue) and related subtrees size/m 51-500 LoC changed system Infrastructure and configuration for the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants