-
Notifications
You must be signed in to change notification settings - Fork 0
Add Multilingual README Support #1
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
Conversation
- Add language code extraction from BCP47 format - Implement automatic language detection based on hass.language - Add backend support detection with intelligent caching - Update repository dashboard and download dialog to pass language - Add automatic reload when language changes - Fully backward compatible with graceful degradation - Add comprehensive documentation and testing guides Related backend PR: hacs/integration#4964
- Fix race condition in backendSupportsLanguage cache: Add promise-based synchronization to prevent concurrent requests from corrupting the cache state. Only set cache if still null to protect against race conditions. - Fix false language change detection: Only refetch repository when oldHass exists and language actually changed, preventing unnecessary API calls on initial property changes. Fixes issues where: 1. Concurrent requests with different languages could corrupt the cache 2. Repository was refetched unnecessarily when oldHass was undefined
…ort it When waiting for a concurrent backend support check, if the backend rejects the language parameter, the code now explicitly removes it from the message object using delete message.language. This prevents the parameter from being sent anyway, which would cause repeated backend errors. Fixes race condition where concurrent requests could leave language parameter in message even after discovering backend doesn't support it.
Ignore debug documentation and test scripts used during development: - DEBUG_WEBSOCKET.md - SAFARI_WEBSOCKET_DEBUGGING.md - test-multilingual-readme.js
When a concurrent request waits for another request's backend support check to complete, if the cache is still null after waiting (indicating the previous request encountered a non-language-related error or didn't complete properly), the code now attempts its own backend support check instead of skipping the language parameter entirely. This prevents a scenario where all concurrent requests give up on determining backend support, causing the cache to never be set and all requests to skip the language parameter regardless of backend capability. Fixes issue where concurrent requests could leave cache in null state permanently if first request failed with non-language-related error.
When waiting for a concurrent backend support check to complete, if the cache is set to true or false, the code was modifying the message object but then falling through to line 223 where it would send a duplicate request. This resulted in sending two requests when only one was needed. Fix: After waiting for concurrent check and setting message based on cache state (true or false), immediately send the request and return, preventing the code from falling through to the final request sending code at line 235. This ensures that: - If cache is true after waiting: send request with language parameter and return - If cache is false after waiting: send request without language parameter and return - If cache is still null after waiting: proceed with our own check (existing logic)
- Remove all language processing logic from frontend - Frontend now only passes hass.language to backend - Remove getBaseLanguageCode() and backend support caching - Simplify fetchRepositoryInformation() to pass language directly
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.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
- Add getBaseLanguageCode() to extract base language from BCP47 format - Enhance fetchRepositoryInformation() to accept optional language parameter - Add language change detection in repository dashboard - Remove unnecessary comments and temporary debug files - Update PULL_REQUEST.md with proper checklist Related to backend PR #4965
Only document changes against main repository, not previous implementation errors
- Remove redundant Features, Behavior, Implementation Details sections - Remove Testing and Type of Change sections - Simplify File Naming Convention - Keep only essential information about changes
- Remove getBaseLanguageCode() function from frontend - Frontend now only passes hass.language directly to backend - All language processing (BCP47 extraction, fallback) handled by backend - Update PULL_REQUEST.md to reflect simplified implementation
- Remove unnecessary if condition in fetchRepositoryInformation - Always set language parameter (uses hass.language as fallback) - Remove obsolete .gitignore entries for deleted debug files
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughFrontend now passes the current Home Assistant language when fetching repository information; repository fetches include language in the websocket payload. The HacsRepositoryDashboard refresh logic now reacts to language changes. The frontend build script was removed and a whitespace change was made to .gitignore. Changes
Sequence DiagramsequenceDiagram
participant UI as HacsRepositoryDashboard / DownloadDialog
participant Data as repository.ts
participant WS as websocket.ts
participant Server as Backend
rect rgb(200,220,255)
Note over UI: Component mounts or Home Assistant language changes
UI->>UI: updated() detects language change / dialog requests info
UI->>Data: fetchRepositoryInformation(hass, repositoryId, language)
end
rect rgb(220,240,220)
Data->>WS: send message including { language: language ?? hass.language, repository_id }
WS->>Server: WebSocket request with language field
Server-->>WS: RepositoryInfo (language-specific)
end
rect rgb(240,220,220)
WS-->>Data: Return RepositoryInfo
Data-->>UI: RepositoryInfo -> UI renders language-specific content
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ 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). (1)
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
♻️ Duplicate comments (1)
src/data/repository.ts (1)
52-63: Language handling still lacks base-code extraction and unsupported-backend fallback
fetchRepositoryInformationnow acceptslanguage, but it still forwardslanguage ?? hass.languageverbatim and always includes thelanguagefield. That means:
- No BCP47 base-code extraction (
"de-DE"→"de") as described in the PR objectives.- No special-casing for English (where you typically want to omit
languageto fall back toREADME.md).- No detection/caching for backends that reject/ignore the
languagefield; older backends will not get the “retry without language” behavior you outlined.To align with the stated behavior and keep the logic centralized here, consider:
Adding a small helper to normalize and filter the language:
const getBaseLanguageCode = (language?: string): string | undefined => { if (!language) return undefined; const base = language.split(/[-_]/, 1)[0].toLowerCase(); // Treat English as default: rely on README.md instead of README.en.md return base === "en" ? undefined : base; };Using it when building the message, only including the field if there is a non-default base code:
export const fetchRepositoryInformation = async ( hass: HomeAssistant, repositoryId: string, language?: string, ): Promise<RepositoryInfo | undefined> => { const baseLanguage = getBaseLanguageCode(language ?? hass.language); const message: any = { type: "hacs/repository/info", repository_id: repositoryId, ...(baseLanguage ? { language: baseLanguage } : {}), }; return hass.connection.sendMessagePromise<RepositoryInfo | undefined>(message); };The additional “detect unsupported backends and stop sending
languagefor this session” logic can then be layered on top of this helper without duplicating normalization.
🧹 Nitpick comments (2)
PULL_REQUEST.md (1)
9-11: Wrap bare URL in a Markdown link to satisfy markdownlintLine 10 uses a bare URL, which triggers MD034. Suggest rewriting as:
- **Backend PR:** [hacs/integration#4965](https://github.com/hacs/integration/pull/4965)This keeps the doc readable and fixes the lint warning.
src/components/dialogs/hacs-download-dialog.ts (1)
330-332: Replace self-assignment reload withlocation.reload()The line
mainWindow.location.href = mainWindow.location.href;is flagged as a self-assignment (
noSelfAssign) and is also less explicit than necessary. You can achieve the same effect more clearly (and appease the linter) with:- mainWindow.location.href = mainWindow.location.href; + mainWindow.location.reload();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.gitignore(1 hunks)PULL_REQUEST.md(1 hunks)script/build(0 hunks)src/components/dialogs/hacs-download-dialog.ts(1 hunks)src/dashboards/hacs-repository-dashboard.ts(1 hunks)src/data/repository.ts(1 hunks)
💤 Files with no reviewable changes (1)
- script/build
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/dialogs/hacs-download-dialog.ts (3)
src/data/repository.ts (5)
RepositoryInfo(41-50)RepositoryBase(12-39)fetchRepositoryInformation(52-64)repositoryDownloadVersion(66-75)repositoryReleases(77-83)src/components/dialogs/show-hacs-dialog.ts (1)
HacsDownloadDialogParams(27-30)src/tools/frontend-resource.ts (1)
generateFrontendResourceURL(3-7)
🪛 Biome (2.1.2)
src/components/dialogs/hacs-download-dialog.ts
[error] 331-331: href is assigned to itself.
This is where is assigned.
Self assignments have no effect and can be removed.
(lint/correctness/noSelfAssign)
🪛 markdownlint-cli2 (0.18.1)
PULL_REQUEST.md
10-10: Bare URL used
(MD034, no-bare-urls)
⏰ 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). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (2)
.gitignore (1)
14-14: LGTM!Adding a trailing newline follows POSIX conventions and is a helpful formatting improvement with no functional impact.
src/components/dialogs/hacs-download-dialog.ts (1)
148-155: Language-aware fetch wiring here looks goodPassing
this.hass.languagethrough tofetchRepositoryInformationmatches the updated signature and keeps the dialog consistent with the dashboard behavior. Once the normalization/fallback infetchRepositoryInformationis fixed, this call-site should not need further changes.
| protected updated(changedProps) { | ||
| super.updated(changedProps); | ||
| if (changedProps.has("repositories") && this._repository) { | ||
| this._fetchRepository(); | ||
| } | ||
| if (changedProps.has("hass") && this._repository) { | ||
| const oldHass = changedProps.get("hass") as HomeAssistant | undefined; | ||
| if (oldHass && oldHass.language !== this.hass.language) { | ||
| this._fetchRepository(); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private async _fetchRepository(repositoryId?: string) { | ||
| try { | ||
| this._repository = await fetchRepositoryInformation( | ||
| this.hass, | ||
| repositoryId || String(this._repository!.id), | ||
| this.hass.language, | ||
| ); | ||
| } catch (err: any) { | ||
| this._error = err?.message; | ||
| } | ||
| } |
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.
updated change detection is partly dead and the parameter is untyped
Two issues in this block:
-
Watching a non-existent property
if (changedProps.has("repositories") && this._repository) { this._fetchRepository(); }
There is no
@propertynamedrepositoriesonHacsRepositoryDashboard, so this condition will never be true. If the intent is to refetch when the global repository list changes, you either need:- A dedicated
@property({ attribute: false }) public repositories!: RepositoryBase[];that the parent updates, and continue watchingrepositories; or - To watch
hacsinstead and derive changes fromthis.hacs.repositories.
As written, this “fix” for repository-change detection is a no-op.
- A dedicated
-
Missing type on
changedPropsdespite importingPropertyValuesupdatedis currently untyped:protected updated(changedProps) {
To avoid implicit
anyand get proper typing support, consider:protected updated(changedProps: PropertyValues<this>): void {
(or at least
PropertyValueswithout the generic if you prefer).
The hass language-change detection itself looks correct; once the repositories handling and typing are fixed, this hook should behave as intended.
🤖 Prompt for AI Agents
In src/dashboards/hacs-repository-dashboard.ts around lines 144 to 167, the
updated() hook is untyped and is checking a non-existent "repositories"
property; change the signature to use PropertyValues (e.g., protected
updated(changedProps: PropertyValues<this>): void) and replace the dead check by
either adding a real @property repositories that the parent will update (e.g.,
@property({ attribute: false }) public repositories!: RepositoryBase[]) if you
intend to watch a prop, or change the condition to watch this.hacs (e.g., if
(changedProps.has("hacs") && this._repository) { /* refetch using
this.hacs.repositories */ }) so repository list changes actually trigger
_fetchRepository; keep the existing hass language-change logic unchanged.
Co-authored-by: gewinkelt-alpha66 <[email protected]>
|
Cursor Agent can help with this pull request. Just |
…() method - Remove accidentally committed PULL_REQUEST.md file - Add proper typing to updated() method with PropertyValues<this> - Remove dead code checking non-existent 'repositories' property
- Remove language parameter from getRepositories() function - Language support is now only for README files via repository/info
Add Multilingual README Support
Summary
This PR adds support for multilingual README files in HACS. The frontend passes the user's Home Assistant language setting (
hass.language) to the backend via thehacs/repository/infoWebSocket command. The backend handles all language processing, file selection, and fallback logic.Related Backend PR
This frontend implementation requires the corresponding backend changes:
Changes
Repository Information Fetching (
src/data/repository.ts)languageparameter tofetchRepositoryInformation()functionlanguagefield in WebSocket message (useshass.languageas fallback)Repository Dashboard (
src/dashboards/hacs-repository-dashboard.ts)hass.languagewhen fetching repository informationupdated()lifecycle hook and automatically refetches repository dataDownload Dialog (
src/components/dialogs/hacs-download-dialog.ts)hass.languagewhen fetching repository informationChecklist
Notes
README.{language_code}.md(e.g.,README.de.md,README.fr.md). The defaultREADME.mdis always used as fallback.Note
Passes hass.language to repository info requests and refetches data when the UI language changes; also removes the legacy build script.
src/data/repository.ts: Add optionallanguageparam tofetchRepositoryInformationand include it in thehacs/repository/infoWebSocket message.src/dashboards/hacs-repository-dashboard.ts: Passhass.languagewhen fetching repository info and refetch if language changes.src/components/dialogs/hacs-download-dialog.ts: Passhass.languageto repository info fetch.script/build.Written by Cursor Bugbot for commit 93ffd45. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.