Ae/minify data #406
Ae/minify data #406
Conversation
…onsResultStatus and translations
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
5 Skipped Deployments
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
Major refactoring of the translation system across all packages, moving from JSX-based content translation to ICU message format while optimizing data structures and improving type safety.
- Introduces
intl-messageformatintegration inpackages/core/src/formatting/format.tsfor standardized message handling - Deprecates JSX translation in favor of ICU format, moving related code to
/deprecateddirectory - Adds dedicated translation status tracking separate from content in provider components
- Upgrades core package to v7.0.0-alpha.13, react to v10.0.0-alpha.13 with breaking changes
- Improves hash generation by replacing
hashJsxChildrenwith more generichashSourcefunction
76 files reviewed, 26 comments
Edit PR Review Bot Settings | Greptile
| children?: any; | ||
| name?: string; | ||
| }): React.JSX.Element { | ||
| function Var({ children }: { children?: any }): React.JSX.Element | null { |
There was a problem hiding this comment.
style: consider using React.ReactNode instead of any for children type to improve type safety
| @@ -10,6 +10,7 @@ import { | |||
| } from 'generaltranslation/internal'; | |||
| import { createUnsupportedLocalesWarning } from '../../errors/createErrors'; | |||
| import { getSupportedLocale } from '@generaltranslation/supported-locales'; | |||
| import { GT } from 'generaltranslation'; | |||
There was a problem hiding this comment.
logic: The GT import is not used anywhere in this file. Remove unused import.
| export class GTTranslationError extends Error { | ||
| constructor( | ||
| public error: string, | ||
| public code: number | ||
| ) { | ||
| super(error); | ||
| this.code = code; | ||
| } |
There was a problem hiding this comment.
logic: constructor is reassigning public parameter 'code' unnecessarily - it's already auto-assigned via the parameter modifier 'public'
| export class GTTranslationError extends Error { | |
| constructor( | |
| public error: string, | |
| public code: number | |
| ) { | |
| super(error); | |
| this.code = code; | |
| } | |
| export class GTTranslationError extends Error { | |
| constructor( | |
| public error: string, | |
| public code: number | |
| ) { | |
| super(error); | |
| } |
| children: number | string | null | undefined; | ||
| name?: string; | ||
| options?: Intl.NumberFormatOptions; | ||
| locales?: string[]; | ||
| }): React.JSX.Element { | ||
| }): React.JSX.Element | null { |
There was a problem hiding this comment.
logic: name prop is defined in type but never used in component parameters. Either remove from type definition or add to parameters if needed.
| children: number | string | null | undefined; | |
| name?: string; | |
| options?: Intl.NumberFormatOptions; | |
| locales?: string[]; | |
| }): React.JSX.Element { | |
| }): React.JSX.Element | null { | |
| children: number | string | null | undefined; | |
| options?: Intl.NumberFormatOptions; | |
| locales?: string[]; |
| children?: any; | ||
| name?: string; | ||
| }): React.JSX.Element { | ||
| function Var({ children }: { children?: any }): React.JSX.Element | null { |
There was a problem hiding this comment.
logic: Return type now includes null but component never returns null. Either remove null type or implement null return case.
| const result = gt | ||
| .formatDateTime(children, { locales, ...options }) | ||
| .replace(/[\u200F\u202B\u202E]/g, ''); |
There was a problem hiding this comment.
style: Consider extracting regex for bidirectional character removal into a named constant for better maintainability
| * @param {Intl.NumberFormatOptions} [options={}] - Optional formatting options for the number, following `Intl.NumberFormatOptions` specifications. | ||
| * @returns {JSX.Element} The formatted number component. | ||
| * @returns {React.JSX.Element} The formatted number component. |
There was a problem hiding this comment.
logic: Return type documentation is incorrect - should include null since component can return null
| * @returns {React.JSX.Element} The formatted number component. | |
| * @returns {React.JSX.Element | null} The formatted number component. |
| let renderedValue: string | number = | ||
| typeof children === 'string' ? parseFloat(children) : children; | ||
| if (typeof renderedValue === 'number') { |
There was a problem hiding this comment.
logic: parseFloat could return NaN which isn't handled. Should add NaN check before formatting
| @@ -18,7 +15,6 @@ import { RuntimeTranslationOptions } from 'gt-react/internal'; | |||
| * | |||
| * @param {string} content - The content string that needs to be translated. | |||
There was a problem hiding this comment.
syntax: param name in JSDoc is 'content' but function uses 'message'
| * @param {string} content - The content string that needs to be translated. | |
| * @param {string} message - The content string that needs to be translated. |
| translation: TranslationSuccess | TranslationLoading | TranslationError | ||
| ): boolean { | ||
| translation: TranslatedChildren | ||
| ) { | ||
| if (!(locale && hash && translation)) return false; |
There was a problem hiding this comment.
logic: setTranslations returns false but the return type is not specified in function signature
| if (!(locale && hash && translation)) return false; | |
| setTranslations( | |
| locale: string, | |
| hash: string, | |
| translation: TranslatedChildren | |
| ): boolean { |
No description provided.