Inject Intl polyfills where used #20798
Conversation
steverep
force-pushed
the
inject-intl-polyfill
branch
from
8bfaeba to
fe48ca5
Compare
WalkthroughWalkthroughThe changes primarily focus on enhancing internationalization support by updating polyfill management and simplifying the codebase. This includes adding new polyfill entries, refining plugin injection logic, and removing redundant imports. Additionally, various modules have been streamlined to directly utilize Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant Component
participant IntlAPI
participant Polyfill
User->>Component: Request for formatted date
Component->>IntlAPI: Check for Intl support
alt Intl supported
IntlAPI-->>Component: Provide formatted date
else Intl not supported
Component->>Polyfill: Load necessary polyfill
Polyfill-->>Component: Provide polyfill methods
Component->>IntlAPI: Use polyfill methods
IntlAPI-->>Component: Provide formatted date
end
Component-->>User: Return formatted date
This diagram illustrates how the component interacts with the Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (4)
Additional Context UsedBiome (11)
Additional comments not posted (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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: 6
Outside diff range and nitpick comments (5)
src/components/ha-language-picker.ts (2)
Line range hint
2-2: Optimize imports by using type-only imports where applicable.- import { css, CSSResultGroup, html, LitElement, PropertyValues } from "lit"; + import type { CSSResultGroup, PropertyValues } from "lit"; + import { css, html, LitElement } from "lit"; - import { customElement, property, query, state } from "lit/decorators"; + import type { PropertyValues } from "lit"; + import { customElement, property, query, state } from "lit/decorators";Also applies to: 9-11
Line range hint
92-95: Avoid non-null assertions to ensure better type safety.- @query("ha-select") private _select!: HaSelect; + @query("ha-select") private _select?: HaSelect;src/data/automation_i18n.ts (3)
Line range hint
2-2: Optimize imports by using type-only imports where applicable.- import { HassConfig } from "home-assistant-js-websocket"; + import type { HassConfig } from "home-assistant-js-websocket"; - import { Condition, ForDict, Trigger } from "./automation"; + import type { Condition, ForDict, Trigger } from "./automation"; - import { DeviceCondition, DeviceTrigger } from "./device_automation"; + import type { DeviceCondition, DeviceTrigger } from "./device_automation"; - import { EntityRegistryEntry } from "./entity_registry"; + import type { EntityRegistryEntry } from "./entity_registry"; - import { FrontendLocaleData } from "./translation"; + import type { FrontendLocaleData } from "./translation";Also applies to: 13-21
Line range hint
57-57: Use template literals for better readability and consistency.- msg += ": " + error.message; + msg = `${msg}: ${error.message}`;Also applies to: 81-81, 713-713
Line range hint
75-75: Specify a more precise type thanany.Consider using a more specific error type or a custom error type that better describes the expected errors in this context.
Also applies to: 707-707
steverep
force-pushed
the
inject-intl-polyfill
branch
from
fe48ca5 to
362015f
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
Outside diff range and nitpick comments (12)
src/components/ha-currency-picker.ts (1)
Line range hint
201-201: Avoid using non-null assertions as they can lead to runtime errors. Consider adding proper null checks or handling the undefined case.- currencyDisplayNames ? currencyDisplayNames.of(currency)! : currency + currencyDisplayNames ? currencyDisplayNames.of(currency) ?? currency : currencysrc/components/ha-country-picker.ts (1)
Line range hint
296-296: It's safer to avoid non-null assertions. Consider handling the potential null case explicitly to prevent runtime errors.- countryDisplayNames.of(country)! + countryDisplayNames.of(country) ?? countryAlso applies to: 303-303
src/common/translations/localize.ts (3)
Line range hint
37-37: Specify more accurate types instead ofanyto enhance type safety and maintainability.- catch (err: any) { + catch (err: unknown) {Also applies to: 55-55, 123-123, 141-141
Line range hint
112-112: Avoid non-null assertions to prevent potential runtime errors. Consider handling the null case explicitly.- cache._localizationCache![messageKey] = translatedMessage; + if (cache._localizationCache) { + cache._localizationCache[messageKey] = translatedMessage; + }Also applies to: 126-126
Line range hint
124-124: Use template literals for string concatenation to improve readability and performance.- return "Translation error: " + err.message; + return `Translation error: ${err.message}`;Also applies to: 148-148
src/components/ha-language-picker.ts (2)
Line range hint
1-1: Optimize imports by using type-only imports where applicable.- import { css, CSSResultGroup, html, LitElement, PropertyValues } from "lit"; + import type { CSSResultGroup, PropertyValues } from "lit"; + import { css, html, LitElement } from "lit";
Line range hint
91-94: Avoid non-null assertions as they can lead to runtime errors if assumptions fail.Consider adding null checks or handling potential null values gracefully to ensure robustness.
src/data/automation_i18n.ts (5)
Line range hint
56-56: Use template literals for better readability and maintainability.- let msg = "Error in describing trigger"; - if (error.message) { - msg += ": " + error.message; - } + let msg = `Error in describing trigger: ${error.message || ''}`;Also applies to: 80-80, 712-712
Line range hint
74-74: Specify a more precise type thananyto improve type safety.Consider using a more specific error type or a custom error interface instead of
anyto enhance type safety and clarity.Also applies to: 706-706
Line range hint
261-261: Avoid non-null assertions to prevent runtime errors.Consider adding null checks or handling potential null values gracefully to ensure robustness.
Also applies to: 1002-1002, 1013-1013
Line range hint
401-401: UseNumber.parseIntinstead of the globalparseIntfor better clarity and safety.- seconds = parseInt(trigger.seconds.substring(1)); + seconds = Number.parseInt(trigger.seconds.substring(1), 10);Also applies to: 402-402, 430-430
Line range hint
405-405: UseNumber.isNaNinstead ofisNaNfor type safety.- if (isNaN(seconds) || seconds > 59 || seconds < 0 || (seconds_interval && seconds === 0)) { + if (Number.isNaN(seconds) || seconds > 59 || seconds < 0 || (seconds_interval && seconds === 0)) {
|
I think there's something broken in the polyfill support for this one, but not sure how I could explain precisely. The polyfill get's injected clearly, but the sensor widgets with a value number or datetime fail to show up. Some errors (multiple of the same error) are throw in this piece of code: frontend/src/common/number/format_number.ts Lines 69 to 82 in f59ed0a Error: Missing locale data for |
|
That try/catch is funky and should be fixed in another PR. What locale are you using? Can you give me steps to reproduce that? |
Legacy build, English locale and setting |
You cannot fully undo the |
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range comments (1)
src/components/ha-country-picker.ts (1)
Line range hint
292-292: Remove non-null assertions to handle potentialundefinedvalues safely.- countryDisplayNames.of(country)! + countryDisplayNames.of(country) ?? 'Unknown Country'Also applies to: 299-299
Proposed change
Injects the
Intlpolyfill where needed using the custom Babel plugin and removes manual imports. I left the import inlocalizefor now (see review comment to potentially remove).This mostly keeps the status quo except automated and injects the polyfill in a few extra places where
Intlis used in dependencies (rrule,chart.js,app-datepicker, etc.). As a follow up, the different polyfills should be broken up into pieces so that each is deferred as much as possible.Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed:
Summary by CodeRabbit
New Features
Intlmethods for better localization and formatting.Refactor
intl-polyfillimports to optimize performance.Bug Fixes
These updates collectively enhance the overall user experience by providing better localization and formatting support while optimizing the application's performance.