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

chore(nuxt): show warning if NuxtPage is not used with pages enabled #25490

Merged
merged 5 commits into from Jan 29, 2024

Conversation

NozomuIkuta
Copy link
Contributor

πŸ”— Linked issue

Resolves #25089

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme or JSdoc annotations)
  • 🐞 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)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

This PR makes Nuxt show a warning message if pages feature is enabled but <NuxtPage /> is never used, like it already does for <NuxtLayout />.

In the original issue, @manniL suggested that swap <RouterView /> in dev mode by a proxy wrapper so that we can insert a logic to be run when RouterView is instantiated.

In this PR, I took another approach that detection logic is inserted into <NuxtPage />, so that component tree can be same between dev and production, and that we can avoid to maintain dev-only component for external library.

Although my approach requires 2 flags (dev-only runtime config and in-component flag), I believe it's simple and reasonable enough. What do you think?

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have added tests (if possible).
  • I have updated the documentation accordingly.

I tested if this works in playground directory.

  • add pages/index.vue with any content
  • Run pnpm run dev πŸ‘‰ warning message is shown
  • Add <RouterView /> to app.vue πŸ‘‰ warning message is shown
  • Replace <RouterView /> with <NuxtPage /> πŸ‘‰ warning message is not shown

Update on documentation is already handled by #25106

Copy link

stackblitz bot commented Jan 29, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

Copy link
Member

@danielroe danielroe left a comment

Choose a reason for hiding this comment

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

Thank you ❀️

I moved the plugin to the pages module to remove that extra runtimeConfig flag, but otherwise πŸ‘Œ

@danielroe danielroe merged commit 0d91e52 into nuxt:main Jan 29, 2024
33 of 34 checks passed
@github-actions github-actions bot mentioned this pull request Jan 29, 2024
@NozomuIkuta NozomuIkuta deleted the chore/router-view-warning branch January 29, 2024 17:26
@robert-gruner
Copy link

@NozomuIkuta We installed v3.10.0 and now we get this warning when running the app in dev mode although we are using NuxtPage in our code normally. Is the check working correctly?

You can see it in a StackBlitz too: https://stackblitz.com/~/edit/github-wgg4gm-sdfzwd

Or are we doing something wrong? Thanks in advance for the info. ✌️

@NozomuIkuta
Copy link
Contributor Author

NozomuIkuta commented Feb 5, 2024

@robert-gruner

Thank you for reporting and providing the reproduction!

This warning seems to work unexpectedly with setPageLayout call.

I created a follow-up issue to track: #25617 πŸ™‹β€β™‚οΈ

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warn in dev mode if <RouterView> is used instead of <NuxtPage>
3 participants