fix: synchronize language state migration across browser tabs - #1175
Conversation
|
@hrshjswniii is attempting to deploy a commit to the itzzavdhesh's projects Team on Vercel. A member of the Team first needs to authorize it. |
✍️ DCO Sign-off NeededHey @hrshjswniii! 👋 One or more commits in this PR are missing a Warning
How to fix: For the latest commit: git commit --amend --signoff
git push --force-with-leaseFor multiple commits, replace git rebase --signoff HEAD~N
git push --force-with-leaseThis comment will update automatically after you push. 🤖 VoiceForge Automation · Updates automatically on edits |
github-actions
Bot
added
assignee-match
bug
🎉 PR Ready for Mentor ReviewHey @hrshjswniii! 👋 Your PR passed all checks and is now in the GSSoC review queue. Note 🔗 Closing: #1132 · 📐 120 lines across 5 file(s) · 📬 Already requested or no eligible reviewer found @sabeenaviklar @Anushreebasics @itsdakshjain @snehkris @Mrigakshi-Rathore @Itzzavdheshh @Nitya-003 @4f4d @lovestaco, this PR is ready for your review — please confirm scope, check behavior and tests, then approve or request changes. Important This is not an approval. Please wait for mentor feedback before expecting a merge. If changes are requested, push them to this same branch and keep the PR focused on the linked issue. 🤖 VoiceForge Automation · Updates automatically on edits |
📝 WalkthroughWalkthroughLanguage persistence now emits custom and storage events, while ChangesLanguage synchronization
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Settings
participant LanguageUtilities
participant Browser
participant Call
participant VoiceForge
Settings->>LanguageUtilities: persistLanguage(language)
LanguageUtilities->>Browser: write storage and dispatch change event
Browser-->>Call: deliver storage or custom event
Browser-->>VoiceForge: deliver storage or custom event
Call->>Call: update language state
VoiceForge->>VoiceForge: update language state
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
client/src/utils/languages.test.js (1)
41-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the cross-tab and cleanup paths explicitly.
This only exercises the same-tab custom event. Add assertions for a relevant
storageevent and for no callback afterunsubscribe(); otherwise the primary cross-tab behavior can regress unnoticed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/utils/languages.test.js` around lines 41 - 54, Add coverage in the “persists language and dispatches custom change event” test for a relevant storage event representing a language change, asserting the subscribed callback receives the new language. After calling unsubscribe, trigger the same event path again and assert the callback is not invoked.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client/src/utils/languages.js`:
- Around line 101-108: Update the language-setting flow around
localStorage.setItem and the custom event dispatch so storage persistence
remains best-effort while window.dispatchEvent runs independently even when
persistence throws. Preserve the existing fallback value, event payload, and
environment/function guards, and keep storage errors from preventing the in-tab
language update.
---
Nitpick comments:
In `@client/src/utils/languages.test.js`:
- Around line 41-54: Add coverage in the “persists language and dispatches
custom change event” test for a relevant storage event representing a language
change, asserting the subscribed callback receives the new language. After
calling unsubscribe, trigger the same event path again and assert the callback
is not invoked.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9b4a5f44-3963-4157-9998-709f4991a982
📒 Files selected for processing (5)
client/src/components/VoiceForge.jsxclient/src/pages/Call.jsxclient/src/pages/Settings.jsxclient/src/utils/languages.jsclient/src/utils/languages.test.js
| try { | ||
| localStorage.setItem(LANGUAGE_STORAGE_KEY, code || "en"); | ||
| const val = code || "en"; | ||
| localStorage.setItem(LANGUAGE_STORAGE_KEY, val); | ||
| if (typeof window !== "undefined" && typeof window.dispatchEvent === "function") { | ||
| window.dispatchEvent(new CustomEvent("voiceforge:languageChanged", { detail: val })); | ||
| } | ||
| } catch { | ||
| // Storage unavailable - continue without persisting. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Dispatch the local event even when storage fails.
A localStorage.setItem quota/security exception exits the shared try before the custom event is dispatched, so other mounted components in the same tab remain stale. Keep storage persistence best-effort, but dispatch the in-tab update independently.
Proposed fix
export function persistLanguage(code) {
+ const val = code || "en";
try {
- const val = code || "en";
localStorage.setItem(LANGUAGE_STORAGE_KEY, val);
- if (typeof window !== "undefined" && typeof window.dispatchEvent === "function") {
- window.dispatchEvent(new CustomEvent("voiceforge:languageChanged", { detail: val }));
- }
} catch {
// Storage unavailable - continue without persisting.
}
+
+ if (
+ typeof window !== "undefined" &&
+ typeof window.dispatchEvent === "function" &&
+ typeof CustomEvent === "function"
+ ) {
+ window.dispatchEvent(new CustomEvent("voiceforge:languageChanged", { detail: val }));
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| localStorage.setItem(LANGUAGE_STORAGE_KEY, code || "en"); | |
| const val = code || "en"; | |
| localStorage.setItem(LANGUAGE_STORAGE_KEY, val); | |
| if (typeof window !== "undefined" && typeof window.dispatchEvent === "function") { | |
| window.dispatchEvent(new CustomEvent("voiceforge:languageChanged", { detail: val })); | |
| } | |
| } catch { | |
| // Storage unavailable - continue without persisting. | |
| export function persistLanguage(code) { | |
| const val = code || "en"; | |
| try { | |
| localStorage.setItem(LANGUAGE_STORAGE_KEY, val); | |
| } catch { | |
| // Storage unavailable - continue without persisting. | |
| } | |
| if ( | |
| typeof window !== "undefined" && | |
| typeof window.dispatchEvent === "function" && | |
| typeof CustomEvent === "function" | |
| ) { | |
| window.dispatchEvent(new CustomEvent("voiceforge:languageChanged", { detail: val })); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/src/utils/languages.js` around lines 101 - 108, Update the
language-setting flow around localStorage.setItem and the custom event dispatch
so storage persistence remains best-effort while window.dispatchEvent runs
independently even when persistence throws. Preserve the existing fallback
value, event payload, and environment/function guards, and keep storage errors
from preventing the in-tab language update.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="client/src/utils/languages.js">
<violation number="1" location="client/src/utils/languages.js:102">
P2: A truthy unsupported code now immediately puts subscribed components into an invalid language state, while a reload resolves the same stored value to `en`. Normalize `val` with `VALID_CODES` before persisting and dispatching so local and cross-tab updates agree.</violation>
<violation number="2" location="client/src/utils/languages.js:104">
P2: The in-tab event dispatch is inside the `try` block that wraps `localStorage.setItem`. If storage throws a quota or security exception (a scenario the existing `catch` already anticipates), the `CustomEvent` is never dispatched and other mounted components in the same tab won't receive the language update — defeating the purpose of this synchronization change.
Move the event dispatch outside the `try`/`catch` so that in-tab notification remains independent of storage success.</violation>
<violation number="3" location="client/src/utils/languages.js:126">
P2: An active older Compose tab changing its legacy key will not update newer tabs once they already have `voiceforge:language`; `loadLanguage()` ignores the new legacy value when the unified key exists. Handle a non-null legacy storage value as the incoming update and migrate/publish it, rather than reloading the existing unified value.</violation>
</file>
<file name="client/src/utils/languages.test.js">
<violation number="1" location="client/src/utils/languages.test.js:13">
P0: All assertions for legacy key migration, localStorage persistence, event dispatching, and cross-tab subscription are silently skipped because the vitest environment is `node` (no `localStorage` or `window` available). The three environmental guards (`if (typeof localStorage !== "undefined")` and `if (typeof window !== "undefined" && typeof window.dispatchEvent === "function")`) wrap the critical assertions, so the tests pass green without ever executing them. The legacy migration test and the event dispatch/subscription test are vacuous — they look like coverage but test nothing.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| @@ -0,0 +1,56 @@ | |||
| import { describe, it, expect, beforeEach, vi } from "vitest"; | |||
There was a problem hiding this comment.
P0: All assertions for legacy key migration, localStorage persistence, event dispatching, and cross-tab subscription are silently skipped because the vitest environment is node (no localStorage or window available). The three environmental guards (if (typeof localStorage !== "undefined") and if (typeof window !== "undefined" && typeof window.dispatchEvent === "function")) wrap the critical assertions, so the tests pass green without ever executing them. The legacy migration test and the event dispatch/subscription test are vacuous — they look like coverage but test nothing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At client/src/utils/languages.test.js, line 13:
<comment>All assertions for legacy key migration, localStorage persistence, event dispatching, and cross-tab subscription are silently skipped because the vitest environment is `node` (no `localStorage` or `window` available). The three environmental guards (`if (typeof localStorage !== "undefined")` and `if (typeof window !== "undefined" && typeof window.dispatchEvent === "function")`) wrap the critical assertions, so the tests pass green without ever executing them. The legacy migration test and the event dispatch/subscription test are vacuous — they look like coverage but test nothing.</comment>
<file context>
@@ -0,0 +1,56 @@
+
+describe("languages.js utility and multi-tab synchronization", () => {
+ beforeEach(() => {
+ if (typeof localStorage !== "undefined") {
+ localStorage.clear();
+ }
</file context>
| export function persistLanguage(code) { | ||
| try { | ||
| localStorage.setItem(LANGUAGE_STORAGE_KEY, code || "en"); | ||
| const val = code || "en"; |
There was a problem hiding this comment.
P2: A truthy unsupported code now immediately puts subscribed components into an invalid language state, while a reload resolves the same stored value to en. Normalize val with VALID_CODES before persisting and dispatching so local and cross-tab updates agree.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At client/src/utils/languages.js, line 102:
<comment>A truthy unsupported code now immediately puts subscribed components into an invalid language state, while a reload resolves the same stored value to `en`. Normalize `val` with `VALID_CODES` before persisting and dispatching so local and cross-tab updates agree.</comment>
<file context>
@@ -99,12 +99,46 @@ export function loadLanguage() {
export function persistLanguage(code) {
try {
- localStorage.setItem(LANGUAGE_STORAGE_KEY, code || "en");
+ const val = code || "en";
+ localStorage.setItem(LANGUAGE_STORAGE_KEY, val);
+ if (typeof window !== "undefined" && typeof window.dispatchEvent === "function") {
</file context>
| const val = code || "en"; | |
| const val = VALID_CODES.has(code) ? code : "en"; |
| function handleStorageEvent(e) { | ||
| if ( | ||
| e.key === LANGUAGE_STORAGE_KEY || | ||
| e.key === "voiceforge:compose-language" || |
There was a problem hiding this comment.
P2: An active older Compose tab changing its legacy key will not update newer tabs once they already have voiceforge:language; loadLanguage() ignores the new legacy value when the unified key exists. Handle a non-null legacy storage value as the incoming update and migrate/publish it, rather than reloading the existing unified value.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At client/src/utils/languages.js, line 126:
<comment>An active older Compose tab changing its legacy key will not update newer tabs once they already have `voiceforge:language`; `loadLanguage()` ignores the new legacy value when the unified key exists. Handle a non-null legacy storage value as the incoming update and migrate/publish it, rather than reloading the existing unified value.</comment>
<file context>
@@ -99,12 +99,46 @@ export function loadLanguage() {
+ function handleStorageEvent(e) {
+ if (
+ e.key === LANGUAGE_STORAGE_KEY ||
+ e.key === "voiceforge:compose-language" ||
+ !e.key
+ ) {
</file context>
| localStorage.setItem(LANGUAGE_STORAGE_KEY, code || "en"); | ||
| const val = code || "en"; | ||
| localStorage.setItem(LANGUAGE_STORAGE_KEY, val); | ||
| if (typeof window !== "undefined" && typeof window.dispatchEvent === "function") { |
There was a problem hiding this comment.
P2: The in-tab event dispatch is inside the try block that wraps localStorage.setItem. If storage throws a quota or security exception (a scenario the existing catch already anticipates), the CustomEvent is never dispatched and other mounted components in the same tab won't receive the language update — defeating the purpose of this synchronization change.
Move the event dispatch outside the try/catch so that in-tab notification remains independent of storage success.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At client/src/utils/languages.js, line 104:
<comment>The in-tab event dispatch is inside the `try` block that wraps `localStorage.setItem`. If storage throws a quota or security exception (a scenario the existing `catch` already anticipates), the `CustomEvent` is never dispatched and other mounted components in the same tab won't receive the language update — defeating the purpose of this synchronization change.
Move the event dispatch outside the `try`/`catch` so that in-tab notification remains independent of storage success.</comment>
<file context>
@@ -99,12 +99,46 @@ export function loadLanguage() {
- localStorage.setItem(LANGUAGE_STORAGE_KEY, code || "en");
+ const val = code || "en";
+ localStorage.setItem(LANGUAGE_STORAGE_KEY, val);
+ if (typeof window !== "undefined" && typeof window.dispatchEvent === "function") {
+ window.dispatchEvent(new CustomEvent("voiceforge:languageChanged", { detail: val }));
+ }
</file context>
Nitya-003
left a comment
Nitya-003
left a comment
There was a problem hiding this comment.
@hrshjswniii Resolve the merge conflicts and comments by bot.
itsdakshjain
left a comment
itsdakshjain
left a comment
There was a problem hiding this comment.
Good work , Resolve the merge conflicts
itzzavdhesh
added
gssoc:approved
🎊 PR Merged SuccessfullyHey @hrshjswniii! 👋 Congratulations and thank you for your contribution to VoiceForge! Note 🔗 Linked issue(s): #1132 · ✅ Marked as merged and complete Maintainers may still handle final cleanup, release notes, or follow-up tracking after the merge. 🤖 VoiceForge Automation · Updates automatically on edits |
4 participants
🚀 Program
GSSoC
📝 Description
This PR resolves language state desynchronization across open browser tabs and components in VoiceForge when users change their output language setting.
Key fixes:
languages.js): RefactoredpersistLanguage(code)inlanguages.jsto dispatch avoiceforge:languageChangedcustom window event upon updating the language setting.subscribeLanguageChange): ExportedsubscribeLanguageChangeinlanguages.jslistening to both localvoiceforge:languageChangedevents and multi-tab browserstorageevents for key migration and synchronization.Call.jsx,VoiceForge.jsx,Settings.jsx): SubscribedCall.jsx,VoiceForge.jsx, andSettings.jsxtosubscribeLanguageChangeso that language updates in any tab update active component states dynamically without requiring a page refresh.languages.test.js): Added Vitest test assertions for language loading, legacy key migration, event dispatching, and synchronization helpers.🔗 Related Issue
Closes #1132
🔄 Type of Change
🧪 How to Test
npm run test --workspace clientand verify all tests pass.📸 Screenshots (if applicable)
✅ Checklist
fix: synchronize language state migration across browser tabs)Summary by cubic
Fixes language desync across tabs and components. Language changes now update instantly everywhere and migrate the legacy
voiceforge:compose-languagekey. Fixes #1132.Bug Fixes
persistLanguagedispatchesvoiceforge:languageChangedafter saving.subscribeLanguageChangelistens to the custom event andstoragechanges (including legacy key and storage clears); returns an unsubscribe.VoiceForge.jsx,Call.jsx, andSettings.jsxsubscribe for live state updates without refresh.Tests
getLanguageByCode.Written for commit a15b6d3. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Tests