Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughinjectTranslations was changed to accept a StreamDocument and to return translation records instead of TemplateProps. The module now delegates translation loading to a new shared getTranslations(locale, instance) which normalizes locales (keeps en-GB/zh-TW, maps zh-Hant → zh-TW), dynamically imports ../../../locales/{instance}/{locale}/visual-editor.json, retries with English on failure, and returns {} when no locale or retries fail. platform.ts and components.ts call getTranslations with "platform" or "components". Inline fallback logic and fallbacks.ts were removed. transformProps callers now return data plus a separate translations field. Sequence Diagram(s)sequenceDiagram
participant Caller as Caller (transformProps / loader)
participant Injector as injectTranslations (StreamDocument)
participant Loader as getTranslations(locale, instance)
participant FS as dynamic import (../../../locales/{instance}/{locale}/visual-editor.json)
Caller->>Injector: injectTranslations(streamDocument)
Injector->>Loader: getTranslations(normalizedLocale, instance)
Loader->>FS: import JSON for computedLocale
alt import succeeds
FS-->>Loader: translations JSON
Loader-->>Injector: translations
else import fails
Loader->>Loader: if not retry and computedLocale != "en" then retry with "en"
Loader->>FS: import en JSON
alt English import succeeds
FS-->>Loader: en translations
Loader-->>Injector: translations (en)
else English import fails
FS-->>Loader: import error
Loader-->>Injector: {}
end
end
Injector-->>Caller: translations (or {})
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](/s/com/githubusercontent/avatars/G.https/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/visual-editor/src/editor/Editor.tsx (1)
159-172: Bug: Race condition check will always fail for regional locales.After normalizing
expectedLocale(e.g.,"en-US"→"en"), line 170 compares the originaltemplateMetadata?.platformLocale("en-US") against the normalizedexpectedLocale("en"). This will never match, sochangeLanguageis never called for any locale with a regional suffix.Store the original locale for the race condition check:
🐛 Proposed fix
const handlePlatformLocaleChange = async () => { if (templateMetadata?.platformLocale) { - let expectedLocale = templateMetadata.platformLocale; + const originalLocale = templateMetadata.platformLocale; + let expectedLocale = originalLocale; // strip off region for translations, except for en-GB and zh-TW if (expectedLocale !== "en-GB" && expectedLocale !== "zh-TW") { expectedLocale = expectedLocale.split("-")[0]; } try { await loadPlatformTranslations(expectedLocale); // Additional check to avoid race conditions when locale changes quickly if ( isCurrent && - templateMetadata?.platformLocale === expectedLocale + templateMetadata?.platformLocale === originalLocale ) { i18nPlatformInstance.changeLanguage(expectedLocale); }
🧹 Nitpick comments (1)
packages/visual-editor/src/editor/Editor.tsx (1)
160-163: Consider adding a brief inline comment explaining why these locales are preserved.Future maintainers would benefit from knowing that
en-GBandzh-TWare exceptions because dedicated translation files exist for them (unlike other regional variants).📝 Suggested comment
let expectedLocale = templateMetadata.platformLocale; - // strip off region for translations, except for en-GB and zh-TW + // Strip off region for translations since most regional variants (e.g., en-US, es-MX) + // don't have dedicated translation files. Preserve en-GB and zh-TW which have their own files. if (expectedLocale !== "en-GB" && expectedLocale !== "zh-TW") { expectedLocale = expectedLocale.split("-")[0]; }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/visual-editor/src/editor/Editor.tsxyext-visual-editor-1.0.5test3.tgz
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-12T21:31:40.196Z
Learnt from: benlife5
Repo: yext/visual-editor PR: 980
File: packages/visual-editor/src/components/pageSections/AboutSection/AboutSectionDetailsColumn.tsx:310-316
Timestamp: 2026-01-12T21:31:40.196Z
Learning: Do not use React hooks inside arbitrary callback functions. Hooks must be called only at the top level of React function components or custom hooks. If a callback (e.g., getItemSummary in a framework context like Puck) is not a component or a custom hook, refactor to move hook usage to a component/hook, or pass necessary data via props/state. This guideline applies to all TSX files in the repo.
Applied to files:
packages/visual-editor/src/editor/Editor.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: call_unit_test / unit_tests (20.x)
- GitHub Check: call_unit_test / unit_tests (24.x)
- GitHub Check: call_unit_test / unit_tests (22.x)
- GitHub Check: semgrep/ci
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
commit: |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/visual-editor/src/utils/i18n/platform.ts (1)
58-74: Move locale normalization before the cache check to prevent repeated imports.Line 59 checks
hasResourceBundlewith the original locale, but line 72 registers understrippedLocale. If a caller passesen-US(a regional variant not in storage), the cache check misses for "en-US", the locale gets normalized to "en", but since the bundle was registered under "en", subsequent calls with "en-US" will still miss the cache and re-import unnecessarily.Normalize first and use the same key for both the cache check and registration:
Suggested fix
export const loadPlatformTranslations = async (locale: string) => { - if (i18nPlatformInstance.hasResourceBundle(locale, NAMESPACE)) { - return; - } - let strippedLocale = locale; if (strippedLocale !== "en-GB" && strippedLocale !== "zh-TW") { strippedLocale = strippedLocale.split("-")[0]; } + if (i18nPlatformInstance.hasResourceBundle(strippedLocale, NAMESPACE)) { + return; + } const translationsToInject = await getTranslations(strippedLocale);
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/visual-editor/src/utils/i18n/components.tspackages/visual-editor/src/utils/i18n/platform.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: call_unit_test / unit_tests (24.x)
- GitHub Check: call_unit_test / unit_tests (20.x)
- GitHub Check: call_unit_test / unit_tests (22.x)
- GitHub Check: create-dev-release
- GitHub Check: semgrep/ci
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/visual-editor/src/utils/i18n/getTranslations.ts`:
- Around line 27-32: The log message in getTranslations.ts is missing a space:
change the template string that currently uses `${instance}translations` to
include a space (e.g., `${instance} translations`) so the error reads "Error
loading <instance> translations for locale"; update the console.error call that
references the instance variable to use the corrected template.
|
Warning: Component files have been updated but no migrations have been added. See https://github.com/yext/visual-editor/blob/main/packages/visual-editor/src/components/migrations/README.md for more information. |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@packages/visual-editor/src/components/atoms/hoursTable.tsx`:
- Around line 30-33: Remove the debug paragraph rendering in the HoursTable
component (packages/visual-editor/src/components/atoms/hoursTable.tsx) that
shows "Current language: {i18n.language}, hours: {t('hours','Hours')}". Delete
that JSX <p> (or wrap it behind a development-only guard like
process.env.NODE_ENV === 'development' if you need it for local debugging) so no
debug UI is rendered in production; keep the rest of the fragment/JSX intact.
In `@packages/visual-editor/src/utils/i18n/components.ts`:
- Line 65: Remove the debug console.log call console.log("loading translations
for locale", locale); from the i18n loading code in
packages/visual-editor/src/utils/i18n/components.ts; either delete the statement
or replace it with the project's standard logger (e.g., logger.debug or
processLogger.debug) if structured debug output is required so no plain console
logs remain.
- Around line 34-45: Remove the two debug console.log calls that print
"injecting translations into templateProps" and "getting translations for
locale" around the locale check; update the logic in the function that processes
templateProps (references: templateProps and templateProps.document.locale) to
either delete those logs or gate them behind an explicit debug flag or logger
(e.g., only call if DEBUG or a passed-in logger is enabled), ensuring the
function returns as before when locale is missing without producing console
output.
In `@packages/visual-editor/src/vite-plugin/templates/main.tsx`:
- Line 119: Remove the debug console.log that prints the full page data: delete
or conditionally guard the call console.log("updatedData",
JSON.stringify(updatedData)) so it is not emitted in production; if you need
runtime debugging keep it behind a development-only flag (e.g.,
process.env.NODE_ENV !== "production" or an explicit debug toggle) referencing
the updatedData variable to avoid logging large documents in production.
|
Ready for review:
With this change, if the locale is es-MX, it will import the es file and inject it into i18next as the es-MX translations, which will then work as expected. Screen.Recording.2026-01-16.at.4.45.03.PM.mov |
4 participants
The browser can set locales such as
en-US,es-MX, etc. but we don't have translation files for these. We should strip the region except foren-GBandzh-TWAlso removes a file I accidentally uploaded 🤦♂️