test/fixtures/basic/app/pages/redirect-target.vue (1)

5-7: Consider adding TypeScript to script setup.

The coding guidelines specify using <script setup lang="ts"> for Vue components. Adding TypeScript would provide type safety and align with the project's conventions.

Apply this diff:

-<script setup>
+<script setup lang="ts">
 const route = useRoute()
 </script>

Based on coding guidelines.

test/fixtures/basic/app/pages/navigate-to-encoded-query.vue (1)

5-9: Consider adding TypeScript to script setup.

The coding guidelines specify using <script setup lang="ts"> for Vue components. Adding TypeScript would provide type safety and align with the project's conventions.

Apply this diff:

-<script setup>
+<script setup lang="ts">
 // This should trigger a redirect to /redirect-target?callback=/other
 // The query parameter contains a path that should be preserved
 await navigateTo('/redirect-target?callback=/other', { redirectCode: 302 })
 </script>

Based on coding guidelines.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

packages/nuxt/src/app/composables/router.ts (1)

217-217: Correct fix for percent-encoding preservation.

Applying encodeURL to both sides of the comparison ensures that paths are normalized to the same format before checking equality. This properly addresses the root cause where fullPath was decoded whilst final.fullPath was percent-encoded, causing the redirect check to fail.

Optional optimisation: Consider caching the encoded target to avoid recomputing on each navigation:

+      const encodedFullPath = encodeURL(fullPath)
-      router.afterEach(final => encodeURL(final.fullPath) === encodeURL(fullPath) ? redirect(false) : undefined)
+      router.afterEach(final => encodeURL(final.fullPath) === encodedFullPath ? redirect(false) : undefined)

This avoids repeatedly calling encodeURL(fullPath) since fullPath is captured in the closure and remains constant across navigation checks. However, the performance impact is negligible given this only runs server-side during middleware processing.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

test/fixtures/basic/app/pages/navigate-to-encoded-query.vue (1)

5-9: Clarify comment to mention percent-encoding.

The comment on line 6 could be more precise to help readers understand the test's purpose. Consider mentioning that the forward slash in /other will be percent-encoded as %2Fother in the query parameter.

Apply this diff to improve the comment:

-// This should trigger a redirect to /redirect-target?callback=/other
+// This should trigger a redirect to /redirect-target?callback=%2Fother (encoded)
 // The query parameter contains a path that should be preserved

Additionally, verify that the corresponding test in test/basic.test.ts properly validates the Location header contains the percent-encoded query parameter (%2Fother).

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

packages/nuxt/src/app/composables/router.ts (2)

217-217: Core fix correctly normalises both paths before comparison.

By encoding both final.fullPath and fullPath before comparing, this change ensures the equality check succeeds even when Vue Router returns them in different encoding states, which was the root cause of the redirect failure.

Optional micro-optimisation: since fullPath is constant in this scope, you could cache its encoded value to avoid encoding it on every afterEach invocation:

+      const encodedFullPath = encodeURL(fullPath)
-      router.afterEach(final => encodeURL(final.fullPath) === encodeURL(fullPath) ? redirect(false) : undefined)
+      router.afterEach(final => encodeURL(final.fullPath) === encodedFullPath ? redirect(false) : undefined)

However, since afterEach is only registered once per navigation and URL encoding is fast, the performance benefit would be negligible.

---

303-338: Consider using ufo's dedicated query encoding functions.

The decode-then-encode pattern for query parameters (lines 307-322) correctly normalises encoding, but manually parsing queries can miss edge cases.

Based on learnings, ufo v1.6.1 provides dedicated functions for query handling that may be more robust:

export function encodeURL (location: string, isExternalHost = false) {
  const url = parseURL(location)
  const pathname = encodePath(url.pathname)
  
  // Use ufo's parseQuery and withQuery for more robust query handling
  const query = url.search ? parseQuery(url.search) : {}
  const search = Object.keys(query).length > 0 
    ? '?' + Object.entries(query)
        .map(([k, v]) => `${encodeURIComponent(k)}=${encodeURIComponent(String(v))}`)
        .join('&')
    : ''
  
  const hash = url.hash ? encodeHash(url.hash) : ''
  
  if (!isExternalHost) {
    return pathname + search + hash
  }
  
  const protocol = url.protocol ? url.protocol + '//' : ''
  const host = url.host || ''
  const result = protocol + host + pathname + search + hash
  
  if (location.startsWith('//')) {
    return result.replace(/^https?:/, '')
  }
  return result
}

This approach leverages ufo's parseQuery (which now uses null-prototype objects for safety) and separates parsing concerns from encoding logic. However, the current implementation is explicit and working correctly, so this refactor is optional.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

packages/nuxt/src/app/composables/router.ts (2)

217-217: LGTM with optional optimization suggestion.

The normalization approach correctly addresses the percent-encoding mismatch issue. Both paths are now encoded consistently before comparison, ensuring redirects work regardless of encoding differences.

For a minor performance improvement, consider caching the encoded target to avoid reparsing on every afterEach invocation:

+      const encodedFullPath = encodeURL(fullPath)
-      router.afterEach(final => encodeURL(final.fullPath) === encodeURL(fullPath) ? redirect(false) : undefined)
+      router.afterEach(final => encodeURL(final.fullPath) === encodedFullPath ? redirect(false) : undefined)

---

331-334: Add comment explaining protocol-relative URL handling.

The special case for URLs starting with // removes the protocol to preserve the protocol-relative format. This works correctly but is non-obvious and would benefit from a brief comment.

   if (location.startsWith('//')) {
+    // Protocol-relative URLs: preserve the '//' prefix by removing any protocol parseURL may have added
     return result.replace(/^https?:/, '')
   }

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro