Skip to content
This repository was archived by the owner on Apr 6, 2023. It is now read-only.
/frameworkPublic archive

refactor(nuxt): explicitly import app in nuxt-root - #8729

Merged
pi0 merged 1 commit into
mainfrom
refactor/app-import
Nov 9, 2022
Merged

refactor(nuxt): explicitly import app in nuxt-root#8729
pi0 merged 1 commit into
mainfrom
refactor/app-import

Conversation

@danielroe

Copy link
Copy Markdown
Member

🔗 Linked issue

❓ Type of change

  • 📖 Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

This is a micro optimisation. We likely don't need access to <App> outside of <NuxtRoot> and it feels like having it in the vue component registry isn't necessary. But I may have missed something here - wdyt @antfu?

📝 Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@danielroe
danielroe requested a review from antfuNovember 5, 2022 13:51
@danielroedanielroe self-assigned this Nov 5, 2022
@codesandbox

Copy link
Copy Markdown

CodeSandbox logoCodeSandbox logo Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

@netlify

netlifyBot commented Nov 5, 2022

Copy link
Copy Markdown

Deploy Preview for nuxt3-docs canceled.

NameLink
🔨 Latest commit81ffdf3
🔍 Latest deploy loghttps://app.netlify.com/sites/nuxt3-docs/deploys/63666a4db791630008d4c1af

@antfuantfu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sounds reasonable to me

@pi0

pi0 commented Nov 7, 2022

Copy link
Copy Markdown
Member

Having App auto import could be useful for custom Root component implementations and also mocking purposes. I think we could rename it to NuxtApp BTW.

Any specific reason made you to make this refactor @danielroe ?

@danielroe

Copy link
Copy Markdown
MemberAuthor

The reason for the refactor is that storing the component in the vue component registry is a slight additional overhead when it's only used once.

For custom root components, we could import directly from #build/app-component.mjs. And it might be easier to add support later for a future use case if we need to do it. And there are some challenges. Really if used, it should be <NuxtRoot> which provides the base <Suspense>. There's no guarantee that the app component will work on its own. At the moment, I think this is an edge case and I know you'd be the first to ask for a really solid use case before enabling. (If we do want to add this feature, I would prefer to use addComponent which would import at point-of-use.)

Regarding name, I think <NuxtApp> would be confusing as it's not the same thing as nuxtApp . <AppComponent> or merely <App> works - but again, it's not quite the same as a normal App.vue in a non-Nuxt project.

@pi0

pi0 commented Nov 9, 2022

Copy link
Copy Markdown
Member

I don't think there is any noticeable overhead of registering one component to the Vue interface (otherwise we have really issues with scaling any Nuxt project with dozens of components...)

Naming NuxtAppComponent makes sense but sure let's add it when we officially support a way to customize Root component. Removing public component until then makes sense 👍🏼

@pi0
pi0 merged commit 3839dba into mainNov 9, 2022
@pi0
pi0 deleted the refactor/app-import branch November 9, 2022 09:14
@pi0pi0 mentioned this pull request Nov 15, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@danielroe@pi0@antfu