feat(i18n): add hook to filter pages for i18n #3726
Conversation
WalkthroughThis update introduces a new Nuxt module hook, ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
specs/fixtures/routing/nuxt.config.ts (1)
11-19: Excellent implementation of the i18n:filterPages hook.The logic correctly identifies admin pages and disables i18n by setting
meta.i18n = false. The defensive initialization ofpage.metaprevents potential errors.Minor suggestion: Consider using
NuxtPage[]instead ofany[]for better type safety, thoughany[]is acceptable for test fixtures.src/pages.ts (1)
86-89: Well-implemented hook invocation with proper defensive programming.The hook call allows external modules to mutate page metadata before localization. The
awaitis appropriate since hooks can be asynchronous, and the defensive check ensures compatibility.Minor suggestion: The
if (nuxt.callHook)check might be unnecessary in a proper Nuxt context, but it's good defensive programming.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
specs/fixtures/routing/nuxt.config.ts(1 hunks)specs/fixtures/routing/pages/admin.vue(1 hunks)src/module.ts(2 hunks)src/pages.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: BobbieGoede
PR: nuxt-modules/i18n#3587
File: src/runtime/plugins/route-locale-detect.ts:24-31
Timestamp: 2025-05-05T20:42:02.900Z
Learning: In nuxt-modules/i18n's route detection, the `detectLocale` function returns a string representing the locale code, not an object. This string can be passed directly to `ctx.setLocale`.
Learnt from: BobbieGoede
PR: nuxt-modules/i18n#3587
File: src/runtime/plugins/route-locale-detect.ts:24-31
Timestamp: 2025-05-05T20:42:02.900Z
Learning: In nuxt-modules/i18n's route detection, the `detectLocale` function returns a string representing the locale code, not an object. This string can be passed directly to `ctx.setLocale`.
specs/fixtures/routing/nuxt.config.ts (2)
Learnt from: BobbieGoede
PR: nuxt-modules/i18n#3587
File: src/runtime/plugins/route-locale-detect.ts:24-31
Timestamp: 2025-05-05T20:42:02.900Z
Learning: In nuxt-modules/i18n's route detection, the `detectLocale` function returns a string representing the locale code, not an object. This string can be passed directly to `ctx.setLocale`.
Learnt from: BobbieGoede
PR: nuxt-modules/i18n#3587
File: src/runtime/plugins/route-locale-detect.ts:24-31
Timestamp: 2025-05-05T20:42:02.900Z
Learning: In nuxt-modules/i18n's route detection, the `detectLocale` function returns a string representing the locale code, not an object. This string can be passed directly to `ctx.setLocale`.
src/module.ts (3)
Learnt from: BobbieGoede
PR: nuxt-modules/i18n#3587
File: src/runtime/plugins/route-locale-detect.ts:24-31
Timestamp: 2025-05-05T20:42:02.900Z
Learning: In the nuxt-modules/i18n package, `_nuxtI18nCtx.setLocale` is a synchronous function that accepts a string locale parameter and returns void. It doesn't need to be awaited when called.
Learnt from: BobbieGoede
PR: nuxt-modules/i18n#3587
File: src/runtime/plugins/route-locale-detect.ts:24-31
Timestamp: 2025-05-05T20:42:02.900Z
Learning: In the nuxt-modules/i18n package, `_nuxtI18nCtx.setLocale` is a synchronous function that accepts a string locale parameter and returns void. It doesn't need to be awaited when called.
Learnt from: BobbieGoede
PR: nuxt-modules/i18n#3584
File: src/runtime/server/context.ts:1-4
Timestamp: 2025-05-04T11:52:41.396Z
Learning: In Nuxt applications, `$fetch` is globally available and doesn't need to be explicitly imported.
src/pages.ts (4)
Learnt from: BobbieGoede
PR: nuxt-modules/i18n#3587
File: src/runtime/plugins/route-locale-detect.ts:24-31
Timestamp: 2025-05-05T20:42:02.900Z
Learning: In nuxt-modules/i18n's route detection, the `detectLocale` function returns a string representing the locale code, not an object. This string can be passed directly to `ctx.setLocale`.
Learnt from: BobbieGoede
PR: nuxt-modules/i18n#3587
File: src/runtime/plugins/route-locale-detect.ts:24-31
Timestamp: 2025-05-05T20:42:02.900Z
Learning: In nuxt-modules/i18n's route detection, the `detectLocale` function returns a string representing the locale code, not an object. This string can be passed directly to `ctx.setLocale`.
Learnt from: BobbieGoede
PR: nuxt-modules/i18n#3587
File: src/runtime/plugins/route-locale-detect.ts:24-31
Timestamp: 2025-05-05T20:42:02.900Z
Learning: In the nuxt-modules/i18n package, `_nuxtI18nCtx.setLocale` is a synchronous function that accepts a string locale parameter and returns void. It doesn't need to be awaited when called.
Learnt from: BobbieGoede
PR: nuxt-modules/i18n#3587
File: src/runtime/plugins/route-locale-detect.ts:24-31
Timestamp: 2025-05-05T20:42:02.900Z
Learning: In the nuxt-modules/i18n package, `_nuxtI18nCtx.setLocale` is a synchronous function that accepts a string locale parameter and returns void. It doesn't need to be awaited when called.
🧬 Code Graph Analysis (2)
src/module.ts (1)
specs/fixtures/routing/nuxt.config.ts (1)
pages(11-19)
src/pages.ts (2)
specs/fixtures/routing/nuxt.config.ts (1)
pages(11-19)src/routing.ts (1)
localizeRoutes(70-123)
🔇 Additional comments (5)
src/module.ts (2)
7-7: LGTM!The addition of
NuxtPageimport is necessary for the new hook signature and correctly placed alongside the existingHookResultimport.
129-129: LGTM!The hook signature is well-defined with proper typing. The
voidreturn type correctly indicates that the hook is intended to mutate the pages array in place rather than return a new array.specs/fixtures/routing/pages/admin.vue (1)
1-3: LGTM!This simple test fixture appropriately demonstrates the i18n filtering functionality for admin pages. The minimal implementation is suitable for its purpose.
src/pages.ts (2)
91-93: Excellent filtering logic with precise condition checking.The filtering correctly excludes only pages with
meta.i18nexplicitly set tofalse, not just falsy values. This preserves pages wheremeta.i18nis undefined or truthy, which is the intended behavior.
95-95: Correct usage of filtered pages for localization.Using
i18nPagesinstead of the fullpagesarray ensures that only pages intended for internationalization are processed bylocalizeRoutes, while keeping excluded pages available for other functionality.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
specs/fixtures/i18n-disable-pages/nuxt.config.ts (1)
10-10: Improve type safety in the hook parameter.The fixture correctly demonstrates the feature, but the
pagesparameter should use proper typing for better type safety.- 'i18n:filterPages'(pages: any[]) { + 'i18n:filterPages'(pages: import('#app').NuxtPage[]) {Alternatively, if the NuxtPage type is not available in this context, you could use a more descriptive interface:
- 'i18n:filterPages'(pages: any[]) { + 'i18n:filterPages'(pages: Array<{ path: string; meta?: { i18n?: boolean } }>) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
specs/fixtures/i18n-disable-pages/nuxt.config.ts(1 hunks)specs/fixtures/i18n-disable-pages/package.json(1 hunks)specs/fixtures/i18n-disable-pages/pages/admin.vue(1 hunks)specs/fixtures/i18n-disable-pages/pages/index.vue(1 hunks)src/kit/gen.ts(1 hunks)src/pages.ts(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- specs/fixtures/i18n-disable-pages/pages/admin.vue
- specs/fixtures/i18n-disable-pages/pages/index.vue
- specs/fixtures/i18n-disable-pages/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages.ts
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: BobbieGoede
PR: nuxt-modules/i18n#3587
File: src/runtime/plugins/route-locale-detect.ts:24-31
Timestamp: 2025-05-05T20:42:02.900Z
Learning: In the nuxt-modules/i18n package, `_nuxtI18nCtx.setLocale` is a synchronous function that accepts a string locale parameter and returns void. It doesn't need to be awaited when called.
Learnt from: BobbieGoede
PR: nuxt-modules/i18n#3587
File: src/runtime/plugins/route-locale-detect.ts:24-31
Timestamp: 2025-05-05T20:42:02.900Z
Learning: In the nuxt-modules/i18n package, `_nuxtI18nCtx.setLocale` is a synchronous function that accepts a string locale parameter and returns void. It doesn't need to be awaited when called.
Learnt from: BobbieGoede
PR: nuxt-modules/i18n#3587
File: src/runtime/plugins/route-locale-detect.ts:24-31
Timestamp: 2025-05-05T20:42:02.900Z
Learning: In nuxt-modules/i18n's route detection, the `detectLocale` function returns a string representing the locale code, not an object. This string can be passed directly to `ctx.setLocale`.
Learnt from: BobbieGoede
PR: nuxt-modules/i18n#3587
File: src/runtime/plugins/route-locale-detect.ts:24-31
Timestamp: 2025-05-05T20:42:02.900Z
Learning: In nuxt-modules/i18n's route detection, the `detectLocale` function returns a string representing the locale code, not an object. This string can be passed directly to `ctx.setLocale`.
src/kit/gen.ts (2)
Learnt from: BobbieGoede
PR: nuxt-modules/i18n#3587
File: src/runtime/plugins/route-locale-detect.ts:24-31
Timestamp: 2025-05-05T20:42:02.900Z
Learning: In nuxt-modules/i18n's route detection, the `detectLocale` function returns a string representing the locale code, not an object. This string can be passed directly to `ctx.setLocale`.
Learnt from: BobbieGoede
PR: nuxt-modules/i18n#3587
File: src/runtime/plugins/route-locale-detect.ts:24-31
Timestamp: 2025-05-05T20:42:02.900Z
Learning: In nuxt-modules/i18n's route detection, the `detectLocale` function returns a string representing the locale code, not an object. This string can be passed directly to `ctx.setLocale`.
specs/fixtures/i18n-disable-pages/nuxt.config.ts (4)
Learnt from: BobbieGoede
PR: nuxt-modules/i18n#3587
File: src/runtime/plugins/route-locale-detect.ts:24-31
Timestamp: 2025-05-05T20:42:02.900Z
Learning: In the nuxt-modules/i18n package, `_nuxtI18nCtx.setLocale` is a synchronous function that accepts a string locale parameter and returns void. It doesn't need to be awaited when called.
Learnt from: BobbieGoede
PR: nuxt-modules/i18n#3587
File: src/runtime/plugins/route-locale-detect.ts:24-31
Timestamp: 2025-05-05T20:42:02.900Z
Learning: In the nuxt-modules/i18n package, `_nuxtI18nCtx.setLocale` is a synchronous function that accepts a string locale parameter and returns void. It doesn't need to be awaited when called.
Learnt from: BobbieGoede
PR: nuxt-modules/i18n#3587
File: src/runtime/plugins/route-locale-detect.ts:24-31
Timestamp: 2025-05-05T20:42:02.900Z
Learning: In nuxt-modules/i18n's route detection, the `detectLocale` function returns a string representing the locale code, not an object. This string can be passed directly to `ctx.setLocale`.
Learnt from: BobbieGoede
PR: nuxt-modules/i18n#3587
File: src/runtime/plugins/route-locale-detect.ts:24-31
Timestamp: 2025-05-05T20:42:02.900Z
Learning: In nuxt-modules/i18n's route detection, the `detectLocale` function returns a string representing the locale code, not an object. This string can be passed directly to `ctx.setLocale`.
🔇 Additional comments (2)
src/kit/gen.ts (1)
101-104: LGTM! Well-implemented early exit condition.The implementation correctly:
- Uses optional chaining for safe property access
- Explicitly checks for
false(not just falsy values)- Returns the original route without localization processing
- Improves performance with early exit before expensive operations
specs/fixtures/i18n-disable-pages/nuxt.config.ts (1)
12-17: Well-structured hook implementation.The logic correctly:
- Iterates through all pages
- Uses appropriate path matching for the demo
- Safely initializes the meta object before setting properties
- Demonstrates the feature clearly
|
Thank you for your PR! After taking a look at the changes in this PR, I'm not sure about adding the Effectively this change https://github.com/nuxt-modules/i18n/pull/3726/files#diff-eb1a20dccd87e92592b66458a2c4d10f5309b3b774fa41bcdc9925f3ad76c1f2R102 would partially enable the |
|
@BobbieGoede This feature skips microcode compilation, so there is no conflict. Is there a priority issue? This priority must be relatively high, right？ |
2 participants
🔗 Linked issue
resolves #3716
📚 Description
Allow modules to modify route localization
Summary by CodeRabbit
New Features
Bug Fixes
Tests