fix(nuxt): skip router middleware/redirections for islands

[harlan-zw]

🔗 Linked issue

nuxt-modules/og-image#98

❓ 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

When rendering a Nuxt Island, we provide the SSR context url as the root route /. Within the rendering pipeline, something is inheriting the route rules for / and applying them. I'm guessing Nitro.

The most obvious issue here is if you have a redirect on your home page, it's going to trigger a redirect on the island render.
You can see a minimal reproduction here: https://stackblitz.com/edit/nuxt-starter-gzuypu?file=pages%2Fabout.vue

While this could probably be dug into to figure out what's causing the redirect and doing an if (!ssrContext.islandContext), I think it's safer for the long term not to re-use this root route.

It's possible that we are using / for a reason, maybe to avoid some 404 issues? Would be good to have your input. The updated URL is arbitrary, ideally, we'd pass no route within the context and rework the engine to support that (or escape early with a specific island renderer).

On that note but unrelated to this PR, I think there are some opportunities to refactor the island renderer to try and decouple it outside of the core logic.

📝 Checklist

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

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (main@b8bfa60). Click here to learn what that means.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #24421   +/-   ##
=======================================
  Coverage        ?   58.65%           
=======================================
  Files           ?        5           
  Lines           ?      861           
  Branches        ?       46           
=======================================
  Hits            ?      505           
  Misses          ?      356           
  Partials        ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[danielroe]

I don't think this is routeRules but rather vue router which is redirecting us, probably because we are still loading the router plugin for use within island.

I'm reluctant to use the event.path as this will still have the same issue with any middleware/redirects that apply to the original path that is loading the component.

Instead, I think we should add some more logic to the router plugin to check we're not in an island context, e.g.:

nuxt/packages/nuxt/src/pages/runtime/plugins/router.ts

Line 146 in 17ff26e

if (import.meta.client || !nuxtApp.ssrContext?.islandContext) {

Later we can perhaps opt-out entirely of routing (or, ideally, provide a 'stub' route rather than fully initialising vue-router). But we need to ensure that useRoute() continues to work within server components so we keep user-land differences to a minimum.

[huang-julien]

Yes this would cause an issue if there's rules applied on /__nuxt_island/**

Is there a possibility to ignore everything for /__nuxt_island ? It implies removing probably both routeRules, nitro global middlewares and the router.

Currently working on server pages, we'll probably need to disable the router plugin and provide a stubbed route for useRoute because it currently doesn't work within islands

[harlan-zw]

Ah yes, that makes sense if Vue Router is handling it. I've pushed up a fix for that.

It's not really clear in this PR but the event.path will always be /__nuxt_island/**, I've added a comment to make that clearer. I have a strong feeling that we should go with this path (assuming there isn't a good reason for using /), it's arbitrary as the user should never interact with it but it does solve another issue and may avoid issues in the future.

For example, if we have a plugin that does something when the user visits the home page directly, it will always get triggered when an island renders, we can work around it using env: { islands: false }, but this seems like an easy way to trip people up.

export default defineNuxtPlugin({
  setup() {
    const route = useRoute()
    if (route.path === '/') {
      // This will run every time an island component renders
    }
  }
})

It feels like we're plugging holes with the implementation so gives us more protection against other possible bugs.

[huang-julien]

@harlan-zw @danielroe what do you think about that ? This is something done on the server page branch. It would allow to be in the same route as the page for an island.
It's still overrideable and this way we won't need to provide any stub for useROute.

[harlan-zw]

I'm good with Daniel's changes, seems good to merge then. Will leave you to resolve them @huang-julien