-
Notifications
You must be signed in to change notification settings - Fork 387
Enhance NextIntl integration to handle multiple scopes within one file #1356
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
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe changes enhance scope range tracking for next-intl to capture per-function translation namespaces. A new Sequence DiagramsequenceDiagram
participant Parser
participant NextIntl
participant RegexUtil
rect rgb(240, 248, 255)
Note over Parser,NextIntl: Scope Detection (Enhanced)
Parser->>NextIntl: getScopeRange(document)
activate NextIntl
NextIntl->>NextIntl: Match useTranslations/getTranslations
NextIntl->>NextIntl: Extract namespace & functionName (e.g., tFoo)
NextIntl->>Parser: NextIntlScopeRange[] (with functionName)
deactivate NextIntl
end
rect rgb(255, 245, 238)
Note over Parser,RegexUtil: Key Extraction (Function-Aware)
Parser->>RegexUtil: handleRegexMatch(scopes, ...)
activate RegexUtil
RegexUtil->>RegexUtil: isNextIntlScopeRange(scope)?
alt NextIntl Scope
RegexUtil->>RegexUtil: Capture nextIntlFunctionName
RegexUtil->>RegexUtil: Match captured name vs scope.functionName
RegexUtil->>RegexUtil: Use adjusted keyIndex if matched
else Generic Scope
RegexUtil->>RegexUtil: Use standard key extraction
end
RegexUtil->>Parser: Extracted keys
deactivate RegexUtil
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (6)
src/utils/Regex.ts (4)
2-9: Fix import order to satisfy import/orderReorder relative/index imports before
~/aliases per linter hints.-import { Config, CurrentFile } from '~/core' -import { isNextIntlScopeRange, NextIntlScopeRange } from '~/frameworks/next-intl' -import i18n from '~/i18n' -import { Log } from '.' -import { KeyInDocument, RewriteKeyContext } from '../core/types' -import { ScopeRange } from '../frameworks/base' -import { QUOTE_SYMBOLS } from '../meta' +import { Log } from '.' +import { KeyInDocument, RewriteKeyContext } from '../core/types' +import { ScopeRange } from '../frameworks/base' +import { QUOTE_SYMBOLS } from '../meta' +import { Config, CurrentFile } from '~/core' +import { isNextIntlScopeRange, NextIntlScopeRange } from '~/frameworks/next-intl' +import i18n from '~/i18n'
35-35: Guard previous-char access when computingquotedAvoid passing
undefinedintoincludes.- const quoted = QUOTE_SYMBOLS.includes(text[start - 1]) + const quoted = QUOTE_SYMBOLS.includes(text[start - 1] ?? '')
65-72: Align parameter type with new union support
handleRegexMatchacceptsScopeRange[] | NextIntlScopeRange[], butregexFindKeysstill narrows toScopeRange[]. Widen for clarity and stronger types.- scopes: ScopeRange[] = [], + scopes: (ScopeRange | NextIntlScopeRange)[] = [],
98-99: Typo:interpated→interpolatedMinor rename for clarity.
- const interpated = i.replace(/{key}/g, Config.regexKey) - return new RegExp(interpated, 'gm') + const interpolated = i.replace(/{key}/g, Config.regexKey) + return new RegExp(interpolated, 'gm')src/frameworks/next-intl.ts (2)
38-50: Allow matches at start-of-line without shifting capture groupsCurrent patterns require a preceding character. Use a non‑capturing group to also match line start while preserving group indexes (1: functionName, 2: key).
- '[^\\w\\d](t(?:[A-Z]\\w*)?)\\s*\\(\\s*[\'"`]({key})[\'"`]', + '(?:^|[^\\w\\d])(t(?:[A-Z]\\w*)?)\\s*\\(\\s*[\'"`]({key})[\'"`]', - '[^\\w\\d](t(?:[A-Z]\\w*)?)\\s*\\.rich\\s*\\(\\s*[\'"`]({key})[\'"`]', + '(?:^|[^\\w\\d])(t(?:[A-Z]\\w*)?)\\s*\\.rich\\s*\\(\\s*[\'"`]({key})[\'"`]', - '[^\\w\\d](t(?:[A-Z]\\w*)?)\\s*\\.markup\\s*\\(\\s*[\'"`]({key})[\'"`]', + '(?:^|[^\\w\\d])(t(?:[A-Z]\\w*)?)\\s*\\.markup\\s*\\(\\s*[\'"`]({key})[\'"`]', - '[^\\w\\d](t(?:[A-Z]\\w*)?)\\s*\\.raw\\s*\\(\\s*[\'"`]({key})[\'"`]', + '(?:^|[^\\w\\d])(t(?:[A-Z]\\w*)?)\\s*\\.raw\\s*\\(\\s*[\'"`]({key})[\'"`]',
99-115: Tighten scopeendto the next declaration for performance and precisionUsing
text.lengthasendmakes scopes span the whole file. You can bound each scope to the next declaration to reduce overlap and scanning.- for (const match of text.matchAll(regex)) { - if (typeof match.index !== 'number') - continue - const variableName = match[1] - const namespace = match[3] - // Add a new scope if a namespace is provided - if (namespace) { - ranges.push({ - start: match.index, - end: text.length, - namespace, - functionName: variableName, - }) - } - } + const matches = [...text.matchAll(regex)] + for (let i = 0; i < matches.length; i++) { + const match = matches[i] + if (typeof match.index !== 'number') + continue + const variableName = match[1] + const namespace = match[3] + const nextIndex = i < matches.length - 1 && typeof matches[i + 1].index === 'number' + ? (matches[i + 1].index as number) + : text.length + if (namespace) { + ranges.push({ + start: match.index, + end: nextIndex, + namespace, + functionName: variableName, + }) + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/frameworks/next-intl.ts(3 hunks)src/utils/Regex.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/utils/Regex.ts (3)
src/core/types.ts (2)
RewriteKeyContext(141-145)KeyInDocument(113-118)src/frameworks/base.ts (1)
ScopeRange(16-20)src/frameworks/next-intl.ts (2)
NextIntlScopeRange(6-8)isNextIntlScopeRange(10-12)
src/frameworks/next-intl.ts (1)
src/frameworks/base.ts (1)
ScopeRange(16-20)
🪛 ESLint
src/utils/Regex.ts
[error] 5-5: . import should occur before import of ~/core
(import/order)
[error] 6-6: ../core/types import should occur before import of ~/core
(import/order)
[error] 7-7: ../frameworks/base import should occur before import of ~/core
(import/order)
[error] 8-8: ../meta import should occur before import of ~/core
(import/order)
[error] 23-23: It's not necessary to initialize 'nextIntlFunctionName' to undefined.
(no-undef-init)
🔇 Additional comments (4)
src/utils/Regex.ts (1)
34-34: Scope matching byfunctionNamelooks correctFiltering NextIntl scopes by
functionNameensures multiple namespaces per file resolve properly. LGTM.src/frameworks/next-intl.ts (3)
6-12: Type guard and extended scope range are solid
NextIntlScopeRange+isNextIntlScopeRangeenable safe runtime narrowing. LGTM.
87-99: API and detection flow look goodReturn type widened to
NextIntlScopeRange[]and regex captures the variable name and namespace. LGTM.
38-50: Confirm naming convention: restrict tot/tPascal?Patterns require
tort+ uppercase. Iftfoo(lowercase) or other names are allowed in your codebase, considert\\w*. If not, ignore.Would you like me to widen the regex to
t\\w*and add tests accordingly?
When trying to use multiple next-intl namespaces in one file, only the last one is used to resolve keys. For example, it used to be:
With this PR, these problems should be fixed so that both
tandtSpecificare resolved to the correct translation previews.Should be fix to #1076, #1226 and #1353