feat!: introduce a way to configure the I18n provider#737
Conversation
Previously, the i18n built-in mechanism managed by the `Translations` class was always in use. It is now possible to plug custom i18n solutions in addition to the built-in implementation. By default, maxGraph now use an implementation that doesn't provide any translation. This reduces the size of the applications that don't want to use the built-in implementation. The bundle size of examples provided in the repository has been reduced by the following values: - js-example: 8kB - ts-example: 5kB BREAKING CHANGES: The built-in `Translations` class is no longer used by default. To use it, call `GlobalConfig.i18n = new TranslationsAsI18n()`.
WalkthroughThis pull request overhauls the internationalization (i18n) mechanism across the codebase. The built-in Changes
Sequence Diagram(s)sequenceDiagram
participant Component
participant GlobalConfig
participant I18nProvider
Component->>GlobalConfig: isI18nEnabled()
GlobalConfig->>I18nProvider: isEnabled()?
I18nProvider-->>GlobalConfig: Boolean result
alt i18n enabled
Component->>GlobalConfig: translate(key, params, default)
GlobalConfig->>I18nProvider: get(key, params, default)
I18nProvider-->>GlobalConfig: translated string
GlobalConfig-->>Component: translated string
else i18n disabled
Component-->>Component: Use default key/string
end
Assessment against linked issues
Possibly related PRs
✨ Finishing Touches
🪧 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
packages/core/src/i18n/provider.ts (1)
36-41: Replace 'Function' type with a more specific function type.The current code uses
Functionas a type for the_callbackparameter, which is discouraged. UsingFunctiontype can lead to type safety issues as it accepts any function-like value.Consider using a more specific type signature:
addResource( _basename: string, _language: string | null, - _callback: Function | null + _callback: ((success: boolean) => void) | null ): void { // do nothing }🧰 Tools
🪛 Biome (1.9.4)
[error] 38-38: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
packages/core/src/serialization/codecs/editor/EditorCodec.ts (1)
200-200: Updated resource addition to use GlobalConfigResource addition now uses
GlobalConfig.i18n.addResourceinstead of the previous directTranslations.addcall. This change correctly adapts the code to the new i18n architecture.However, note the use of the non-null assertion operator (
!) - while this preserves existing behavior, consider adding a null check for better robustness in future updates.packages/core/src/view/handler/ElbowEdgeHandler.ts (1)
133-133: Updated tooltip text translationThe code now uses the
translatefunction for tooltip translation, correctly implementing the new i18n approach. This allows the tooltip text to be translated via any configured i18n provider.The trailing comment
// translateappears to be unnecessary and could be removed for cleaner code.- tip = translate(tip) || tip; // translate + tip = translate(tip) || tip;packages/core/src/view/Graph.ts (1)
394-396: Updated check for validation errors resourceThe code now uses the
isI18nEnabled()function to determine whether to set the resource key for the validation errors message. This correctly implements the new centralized i18n configuration approach.The multi-line formatting is different from the single-line formatting used for
alreadyConnectedResource. While functionally identical, for consistency you might consider using the same formatting style for both.- containsValidationErrorsResource: string = isI18nEnabled() - ? 'containsValidationErrors' - : ''; + containsValidationErrorsResource: string = isI18nEnabled() ? 'containsValidationErrors' : '';packages/core/src/i18n/Translations.ts (2)
383-412: NewTranslationsAsI18nadapter classThis class effectively implements the
I18nProviderinterface by wrapping the static methods ofTranslations, allowing seamless integration with the new pluggable i18n system. This is a good example of the adapter pattern to maintain backward compatibility while enabling new functionality.Consider using a more specific function type signature instead of the generic
Functiontype for better type safety. For example:- addResource( - basename: string, - language: string | null, - callback: Function | null - ): void { + addResource( + basename: string, + language: string | null, + callback: (() => void) | null + ): void {🧰 Tools
🪛 Biome (1.9.4)
[error] 408-408: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
80-381: Consider refactoring static-only classStatic-only classes like
Translationstypically indicate an opportunity for refactoring to a more functional approach or a singleton pattern.Consider refactoring the
Translationsclass to use a namespace object or module of functions rather than a class with only static members. This would better align with modern JavaScript/TypeScript practices:export namespace Translations { // Current static properties and methods export const resources: { [key: string]: string } = {}; export const extension = '.txt'; // etc. }🧰 Tools
🪛 Biome (1.9.4)
[error] 80-381: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
[error] 192-192: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
[error] 376-376: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
CHANGELOG.md(1 hunks)packages/core/src/editor/Editor.ts(10 hunks)packages/core/src/editor/EditorPopupMenu.ts(2 hunks)packages/core/src/gui/MaxForm.ts(3 hunks)packages/core/src/gui/MaxWindow.ts(3 hunks)packages/core/src/i18n/Translations.ts(4 hunks)packages/core/src/i18n/provider.ts(1 hunks)packages/core/src/index.ts(1 hunks)packages/core/src/internal/i18n-utils.ts(1 hunks)packages/core/src/serialization/codecs/editor/EditorCodec.ts(3 hunks)packages/core/src/serialization/codecs/editor/EditorToolbarCodec.ts(2 hunks)packages/core/src/types.ts(2 hunks)packages/core/src/util/config.ts(3 hunks)packages/core/src/view/Graph.ts(2 hunks)packages/core/src/view/GraphSelectionModel.ts(2 hunks)packages/core/src/view/GraphView.ts(2 hunks)packages/core/src/view/geometry/node/StencilShape.ts(2 hunks)packages/core/src/view/handler/ElbowEdgeHandler.ts(3 hunks)packages/core/src/view/mixins/FoldingMixin.ts(2 hunks)packages/core/src/view/mixins/TooltipMixin.ts(2 hunks)packages/core/src/view/mixins/ValidationMixin.ts(3 hunks)packages/core/src/view/other/Multiplicity.ts(2 hunks)packages/html/.storybook/preview.ts(3 hunks)packages/website/docs/usage/i18n.md(2 hunks)
🧰 Additional context used
🧬 Code Definitions (15)
packages/core/src/view/GraphView.ts (1)
packages/core/src/internal/i18n-utils.ts (1)
isI18nEnabled(22-24)
packages/core/src/view/handler/ElbowEdgeHandler.ts (1)
packages/core/src/internal/i18n-utils.ts (2)
isI18nEnabled(22-24)translate(29-35)
packages/core/src/view/GraphSelectionModel.ts (1)
packages/core/src/internal/i18n-utils.ts (1)
isI18nEnabled(22-24)
packages/core/src/i18n/provider.ts (1)
packages/core/src/types.ts (1)
I18nProvider(1315-1347)
packages/core/src/editor/EditorPopupMenu.ts (1)
packages/core/src/internal/i18n-utils.ts (1)
translate(29-35)
packages/core/src/view/geometry/node/StencilShape.ts (1)
packages/core/src/internal/i18n-utils.ts (1)
translate(29-35)
packages/core/src/view/mixins/FoldingMixin.ts (1)
packages/core/src/internal/i18n-utils.ts (1)
isI18nEnabled(22-24)
packages/core/src/view/other/Multiplicity.ts (1)
packages/core/src/internal/i18n-utils.ts (1)
translate(29-35)
packages/core/src/view/mixins/TooltipMixin.ts (1)
packages/core/src/internal/i18n-utils.ts (1)
translate(29-35)
packages/core/src/view/mixins/ValidationMixin.ts (1)
packages/core/src/internal/i18n-utils.ts (1)
translate(29-35)
packages/core/src/serialization/codecs/editor/EditorToolbarCodec.ts (1)
packages/core/src/internal/i18n-utils.ts (1)
translate(29-35)
packages/core/src/view/Graph.ts (1)
packages/core/src/internal/i18n-utils.ts (1)
isI18nEnabled(22-24)
packages/core/src/editor/Editor.ts (1)
packages/core/src/internal/i18n-utils.ts (2)
isI18nEnabled(22-24)translate(29-35)
packages/core/src/util/config.ts (2)
packages/core/src/i18n/provider.ts (1)
NoOpI18n(26-42)packages/core/src/types.ts (1)
I18nProvider(1315-1347)
packages/core/src/i18n/Translations.ts (3)
packages/core/src/internal/utils.ts (1)
isNullish(39-40)packages/core/src/types.ts (1)
I18nProvider(1315-1347)packages/core/src/i18n/config.ts (1)
TranslationsConfig(60-142)
🪛 Biome (1.9.4)
packages/core/src/i18n/provider.ts
[error] 38-38: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
packages/core/src/types.ts
[error] 1345-1345: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
packages/core/src/i18n/Translations.ts
[error] 80-381: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
[error] 408-408: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
🔇 Additional comments (53)
packages/core/src/i18n/provider.ts (1)
26-33: LGTM! The NoOpI18n implementation correctly implements the I18nProvider interface.The class provides a null implementation that always returns false for
isEnabled()and null forget(), which matches the objective of providing a minimal non-translating provider by default.packages/core/src/editor/EditorPopupMenu.ts (1)
195-195: LGTM! Properly migrated from Translations.get to the new translate function.The code has been correctly updated to use the new i18n system, maintaining the same fallback behavior if translation is not available.
packages/core/src/view/other/Multiplicity.ts (2)
22-22: Good import change for the new i18n utility.The import statement has been correctly updated to use the new i18n-utils module.
63-64: LGTM! Correctly migrated from Translations.get to the new translate function.Both error message properties now use the new
translatefunction with appropriate fallbacks if translations aren't available.packages/core/src/view/GraphView.ts (2)
49-49: Good import update for the new i18n utility.The import statement has been correctly updated to use the new i18n-utils module.
125-125:Details
❓ Verification inconclusive
LGTM! Resource keys now use the new isI18nEnabled function.
The code has been correctly updated to use the new internationalization system's enablement check.
Since this is part of a breaking change where the built-in Translations class is no longer used by default, verify that applications using these resource keys have been updated to explicitly set the i18n provider:
Also applies to: 133-133
🌐 Web query:
What configuration is needed to maintain i18n functionality after maxGraph 0.17.0?💡 Result:
To maintain i18n functionality in maxGraph after version 0.17.0, developers should implement the following configurations based on recent framework updates:
Internationalization Configuration
Core Implementation
- Use the newly introduced
TranslationsConfigobject to manage localized resources and translation mappings [1]- Load default resource bundles now included for Chinese, English, German, French, and Spanish [1]
import { TranslationsConfig } from '@maxgraph/core'; // Configure default language resources TranslationsConfig.setDefaultLanguage('en'); TranslationsConfig.registerResourceBundle('es', spanishTranslations);Customization Requirements
- Create extension points in
CellRendererto handle custom overlay shapes and their DOM localization [1]- Prepare custom resource bundles following the pattern established in restored mxGraph resource files [1]
Compatibility Considerations
- Monitor GitHub issue #688 ("Allow custom i18n mechanisms") for upcoming API changes to custom translation systems [2]
- Check connector configuration changes (
ManhattanConnectorConfigandOrthogonalConnectorConfig) that might affect localized diagram layouts [1]Developers should review the official
maxgraph_0.16.0_website.zipdocumentation package for updated implementation examples and watch for 0.17.0 release notes regarding potential tree-shaking optimizations affecting resource loading [1][2]. The framework now supports easier future i18n extensions through centralized configuration management, though custom translation systems may require additional setup in subsequent versions based on ongoing development efforts [1][2].Citations:
- 1: https://github.com/maxGraph/maxGraph/releases
- 2: https://github.com/maxGraph/maxGraph/issues
- 3: https://github.com/maxGraph/maxGraph/blob/main/README.md
- 4: https://formulae.brew.sh/formula/
Configuration Update Needed: Verify i18n Provider Setup
- LGTM on the change to use
isI18nEnabled()in both GraphView.ts lines 125 and 133.- Since maxGraph 0.17.0 no longer uses the built-in Translations class by default, please ensure that your application explicitly sets up the i18n provider.
- Specifically, verify that you are configuring internationalization via the new
TranslationsConfig—loading default resource bundles (e.g., for Chinese, English, German, French, and Spanish) and registering any custom bundles as needed.packages/core/src/view/mixins/TooltipMixin.ts (2)
23-23: Good addition of the necessary import.The import of the
translatefunction from i18n-utils is correctly added to replace the previous translation mechanism.
43-43: Well implemented transition to configurable i18n.The replacement of
Translations.get(tip)withtranslate(tip)is correctly implemented, maintaining the same functionality while allowing for custom i18n providers. This change aligns well with the PR objective of making i18n configurable.CHANGELOG.md (1)
15-15: Clear documentation of breaking change.This changelog entry correctly informs users about the breaking change to the i18n mechanism and provides clear instructions on how to maintain the previous behavior by setting
GlobalConfig.i18n = new TranslationsAsI18n().packages/html/.storybook/preview.ts (3)
19-19: Good addition of TranslationsAsI18n import.The import is correctly added to support the explicit configuration of the i18n provider.
31-31: Well implemented i18n provider configuration.Creating an instance of TranslationsAsI18n ensures the storybook examples continue to work with translations as before, which is a good example of how consumers should update their code to maintain translation functionality.
42-42: Properly configured i18n in reset function.Setting the i18n provider in the resetMaxGraphConfigs function ensures consistent behavior across all storybook examples. This is exactly how users of the library should configure i18n in their applications.
packages/core/src/view/GraphSelectionModel.ts (2)
26-26: Good update to use the new i18n utility function.The import is correctly updated to use the new
isI18nEnabledfunction from i18n-utils.
81-81: Consistent implementation of new i18n checking.The replacement of
TranslationsConfig.isEnabled()withisI18nEnabled()is correctly implemented for both resource properties. This change maintains the same functionality while adapting to the new i18n configuration model.Also applies to: 89-89
packages/core/src/internal/i18n-utils.ts (1)
1-35: Well-structured utility functions for i18nThe implementation of these utility functions provides a clean abstraction over the i18n provider configured in GlobalConfig. The functions correctly delegate to the configured provider and have appropriate TypeScript typing.
packages/core/src/util/config.ts (2)
17-18: LGTM: Clean import additionsThe imports for I18nProvider and NoOpI18n are correctly added, maintaining good code organization.
Also applies to: 25-26
36-52: Thorough documentation for the new i18n configurationThe JSDoc for the new i18n property is well-documented with clear explanations of available implementations and usage examples. This will help users understand how to configure internationalization in their applications.
packages/core/src/gui/MaxForm.ts (3)
19-23: LGTM: Import replaced correctlyThe direct import of Translations has been correctly replaced with the new translate utility function.
69-69: Good fallback handling for translationsThe implementation correctly uses the translate function with a fallback to 'OK' when translation is not available, ensuring the button always has a label.
78-78: Good fallback handling for translationsSimilar to the OK button, the Cancel button correctly uses translate with a fallback to ensure there's always a visible label.
packages/core/src/view/mixins/FoldingMixin.ts (2)
26-26: LGTM: Import updated correctlyThe TranslationsConfig import has been properly replaced with the isI18nEnabled utility function.
66-66: Functionality preserved with new i18n utilityThe collapseExpandResource property now correctly uses isI18nEnabled() instead of TranslationsConfig.isEnabled(), maintaining the same behavior while adopting the new i18n architecture.
packages/core/src/view/geometry/node/StencilShape.ts (2)
35-35: Import change aligns with new i18n strategyThe import statement has been updated to use the new
translatefunction from the i18n-utils module, which is part of the broader i18n refactoring effort.
178-178: Updated to use the new translation mechanismThe code now uses the
translatefunction instead of the previous directTranslations.getcall, which is the correct implementation for the new i18n approach. This change will use the globally configured i18n provider for translation, making the internationalization process more flexible.packages/core/src/serialization/codecs/editor/EditorCodec.ts (2)
23-24: New imports support the revised i18n strategyThe imports of
GlobalConfigandtranslateare added to support the new centralized i18n configuration mechanism, replacing the direct use of theTranslationsclass.
175-176: Migrated window title translation to the new i18n systemThe code now uses the
translatefunction for retrieving the translated window title, properly implementing the new i18n approach. This allows for any configured i18n provider to be used, instead of hardcoding to theTranslationsclass.packages/core/src/view/handler/ElbowEdgeHandler.ts (2)
28-28: Import updated for new i18n utilitiesThe import statement has been updated to use both the
isI18nEnabledandtranslatefunctions from the i18n-utils module, which is in line with the broader i18n refactoring.
59-59: Updated internationalization checkThe code now uses the
isI18nEnabled()function to determine whether to set the resource key for tooltip translation. This change correctly implements the new centralized i18n configuration approach.packages/core/src/view/Graph.ts (2)
63-63: Import for the new i18n utilityThe import of
isI18nEnabledfrom the i18n-utils module has been added, supporting the broader i18n refactoring effort.
386-386: Updated check for already connected resourceThe code now uses the
isI18nEnabled()function to determine whether to set the resource key for the "already connected" message. This correctly implements the new centralized i18n configuration approach.packages/core/src/serialization/codecs/editor/EditorToolbarCodec.ts (1)
147-147: Good i18n migration implementation.The change from
Translations.get(as)totranslate(as) || asproperly implements the new i18n approach while maintaining the original fallback behavior. This ensures that if the translation isn't found, the original key will be used as before.packages/core/src/types.ts (2)
17-17: Removed unused import.Good cleanup by removing the unused
DIRECTIONimport while keeping the necessaryIDENTITY_FIELD_NAMEimport.
1308-1347: Well-designed newI18nProviderinterface.The new interface defines a clean contract for i18n implementations. Marking it as experimental is appropriate since this is a new pattern that might evolve.
There's a potential issue with using the generic
Functiontype on line 1345. Consider using a more specific function type definition:- callback?: Function | null + callback?: (() => void) | nullOr if parameters are needed:
- callback?: Function | null + callback?: ((success: boolean, ...args: any[]) => void) | null🧰 Tools
🪛 Biome (1.9.4)
[error] 1345-1345: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
packages/core/src/view/mixins/ValidationMixin.ts (3)
20-20: Correct import for the new i18n system.The import of
translatefrom the i18n-utils module is correctly added to replace the removed Translations import.
92-94: Good translation implementation with fallback.The implementation correctly uses the new
translatefunction while maintaining the fallback to the original resource string if translation fails, ensuring backward compatibility.
175-177: Good translation implementation with fallback.Similar to the earlier change, this correctly implements the new i18n approach with proper fallback behavior.
packages/core/src/index.ts (2)
127-127: Correctly exports new TranslationsAsI18n class.Good update to export both the original Translations class and the new TranslationsAsI18n adapter class, making it available for users who want to explicitly configure i18n.
129-129: Complete export of i18n provider module.This export ensures all components from the provider module are available to users, which is essential for the new configurable i18n system.
packages/core/src/gui/MaxWindow.ts (3)
31-31: Import alignment with new i18n architectureThe addition of this import is part of the new i18n architecture, replacing direct usage of the
Translationsclass with the more flexibletranslatefunction from the i18n utility module.
1061-1061: Added fallback for translationGood improvement by adding a fallback pattern (
|| utils.errorResource) to ensure a value is always displayed even when translations aren't available or configured. This aligns with the new default no-op i18n implementation.
1083-1083: Consistent translation fallback patternThis maintains the same fallback pattern as the previous change, ensuring consistent behavior throughout the codebase. This is a good approach for graceful degradation when translations are not available.
packages/website/docs/usage/i18n.md (3)
6-22: Well-structured introduction to the new i18n architectureThis introduction clearly explains the new pluggable i18n mechanism and the reasoning behind the changes. The warning block effectively communicates the important change that translations are no longer enabled by default, which is a breaking change users need to be aware of.
24-31: Clear instructions for using the built-in providerThis section provides straightforward instructions for using the built-in provider, which is essential for users who want to maintain the previous functionality. The code example is clear and concise.
84-97: Good guidance for implementing custom providersThis section effectively explains how to implement and register a custom i18n provider, which is valuable for applications that already have their own translation systems. The instructions are clear and easy to follow.
packages/core/src/editor/Editor.ts (5)
61-61: Updated imports for the new i18n architectureImporting both the
isI18nEnabledcheck and thetranslatefunction ensures proper integration with the new i18n system.
448-448: Conditional resource keys based on i18n availabilityThese changes ensure that resource keys are only set when i18n is enabled, which helps reduce unnecessary translation lookups when the feature isn't being used. This alignment with the new architecture will help optimize bundle size as stated in the PR objectives.
Also applies to: 459-459, 466-466, 474-474, 482-482, 490-490, 498-498
1233-1234: Translation with fallback for user promptsThis properly implements the translation function with a fallback to ensure the UI remains functional even without translations.
1719-1720: Status message translations with consistent fallback patternThese changes maintain a consistent approach to translation throughout the application, ensuring that status messages are properly internationalized when enabled.
Also applies to: 1726-1729
2041-2041: Window title translations with fallbackThese changes ensure proper translation of window titles with appropriate fallbacks, completing the transition to the new i18n system throughout the UI components.
Also applies to: 2221-2221, 2300-2300, 2354-2354
packages/core/src/i18n/Translations.ts (4)
22-25: Enhanced imports for improved type safetyThe addition of the
isNullishutility andI18nProviderinterface improves type safety and code readability throughout the internationalization system.
80-80: Changed to default export for better import patternsMaking
Translationsa default export simplifies imports in other files and follows modern JavaScript module patterns.🧰 Tools
🪛 Biome (1.9.4)
[error] 80-381: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
318-323: Improved type safety in thegetmethodMaking the
keyparameter nullable with a default value allows for more flexible usage patterns and better type safety. The updated implementation properly handles the nullable key case.
325-334: Enhanced null checking withisNullishUsing the
isNullishutility function improves robustness by checking for bothnullandundefinedvalues, preventing potential runtime errors.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/core/src/types.ts (1)
1345-1345: Avoid using the genericFunctiontypeThe
Functiontype is too generic and can lead to type safety issues. Consider using a more specific function signature for the callback parameter.addResource( basename?: string | null, language?: string | null, - callback?: Function | null + callback?: (() => void) | null ): void;🧰 Tools
🪛 Biome (1.9.4)
[error] 1345-1345: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
packages/core/src/i18n/Translations.ts (3)
192-192: Avoid usingFunctionas a type.The
Functiontype is too generic and accepts any function-like value, which can lead to type safety issues. Consider using a more specific function type signature.- callback: Function | null = null + callback: (() => void) | null = null🧰 Tools
🪛 Biome (1.9.4)
[error] 192-192: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
408-408: Avoid usingFunctionas a type.The
Functiontype is too generic and accepts any function-like value, which can lead to type safety issues. Consider using a more specific function type signature.- callback?: Function | null + callback?: (() => void) | null🧰 Tools
🪛 Biome (1.9.4)
[error] 408-408: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
80-381: Consider refactoring static-only class.The
Translationsclass contains only static members, which is generally considered an anti-pattern in object-oriented design. Consider refactoring this into a namespace or a collection of utility functions.However, since this is an established API and part of a larger system, this refactoring should be considered for a future major version change.
🧰 Tools
🪛 Biome (1.9.4)
[error] 80-381: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
[error] 192-192: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
[error] 376-376: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/core/src/editor/Editor.ts(10 hunks)packages/core/src/i18n/Translations.ts(5 hunks)packages/core/src/i18n/provider.ts(1 hunks)packages/core/src/serialization/codecs/editor/EditorCodec.ts(3 hunks)packages/core/src/types.ts(2 hunks)packages/core/src/view/handler/ElbowEdgeHandler.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/core/src/i18n/provider.ts
- packages/core/src/view/handler/ElbowEdgeHandler.ts
- packages/core/src/serialization/codecs/editor/EditorCodec.ts
🧰 Additional context used
🧬 Code Definitions (1)
packages/core/src/editor/Editor.ts (1)
packages/core/src/internal/i18n-utils.ts (2)
isI18nEnabled(22-24)translate(29-35)
🪛 Biome (1.9.4)
packages/core/src/types.ts
[error] 1345-1345: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
packages/core/src/i18n/Translations.ts
[error] 80-381: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
[error] 192-192: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
[error] 408-408: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
🔇 Additional comments (17)
packages/core/src/types.ts (1)
1307-1347: New I18nProvider interface for configurable internationalization supportThis new interface defines the core contract for internationalization providers, allowing the system to be extended with custom i18n solutions. It provides methods to check if translations are enabled, retrieve translated strings, and load translation resources.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1345-1345: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
packages/core/src/editor/Editor.ts (10)
60-60: Import new i18n utility functionsThe PR adds new internationalization utility functions to replace direct calls to the Translations class.
447-497: Update resource keys to use the new i18n enabled checkThe resource key properties are now conditionally set based on
isI18nEnabled(), making them empty strings when i18n is disabled. This change supports the new configurable i18n approach, where i18n functionality is no longer enabled by default.
1232-1233: Replace direct translation call with new translate functionUpdated to use the new centralized
translate()function which delegates to the configured i18n provider.
1718-1720: Update status container to use the translate functionThe status container now uses the
translate()function to get localized strings for the last saved timestamp.
1725-1728: Update current file display to use the translate functionThe current file display now uses the
translate()function to get localized strings for the current file message.
2040-2041: Use translate function for properties window titleUpdated to use the new centralized
translate()function for the properties window title.
2220-2221: Use translate function for tasks window titleUpdated to use the new centralized
translate()function for the tasks window title.
2289-2290: Use translate function for help URLUpdated to use the new centralized
translate()function to retrieve the help URL, with a fallback to the class property.
2299-2300: Use translate function for help window titleUpdated to use the new centralized
translate()function for the help window title.
2353-2354: Use translate function for outline window titleUpdated to use the new centralized
translate()function for the outline window title.packages/core/src/i18n/Translations.ts (6)
189-196: Improved parameter nullability inaddmethod.The
basenameparameter now acceptsnullwith a default value, which is more flexible. However, the method checks ifbasenameis not nullish before proceeding, which means passingnullexplicitly is equivalent to not providing the parameter.🧰 Tools
🪛 Biome (1.9.4)
[error] 192-192: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
318-324: Improved parameter nullability ingetmethod.The
keyparameter now acceptsnullwith a default value, and the method handles this case by setting the value tonullwhen the key is falsy. This makes the API more flexible, though it means the method can no longer distinguish between a missing key and anullkey.
325-328: Enhanced null handling withisNullish.Using the
isNullishutility function improves the robustness of the code by consistently handling bothnullandundefinedcases.
330-333: Enhanced null handling for placeholder replacement.The condition for replacing placeholders now checks if
valueis not nullish before attempting replacement, preventing potential errors when the value isnullorundefined.
383-412:Details
✅ Verification successful
New
TranslationsAsI18nadapter class supports the new i18n provider architecture.This class nicely implements the new
I18nProviderinterface while delegating to the staticTranslationsmethods. The design pattern used here (Adapter) is appropriate for maintaining backward compatibility while introducing a new architecture.Note that as mentioned in the PR objectives, users now need to explicitly set
GlobalConfig.i18n = new TranslationsAsI18n()to use the built-in translations.
🏁 Script executed:
#!/bin/bash # Check for documentation about this breaking change rg -A 3 "GlobalConfig.i18n\s*=\s*new\s*TranslationsAsI18n" --type mdLength of output: 506
New
TranslationsAsI18nadapter confirmed and documentation verifiedThis adapter correctly implements the new
I18nProviderinterface by delegating to the staticTranslationsmethods, effectively using the Adapter pattern to maintain backward compatibility. The breaking change—users now needing to setGlobalConfig.i18n = new TranslationsAsI18n()—is properly documented in both the CHANGELOG.md and the usage docs (packages/website/docs/usage/i18n.md).🧰 Tools
🪛 Biome (1.9.4)
[error] 408-408: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
80-80:Details
❓ Verification inconclusive
Export change makes this a breaking import change.
The class is now exported as default, which means imports need to be updated from
import { Translations } from '...'toimport Translations from '...'. This change should be documented in the migration guide, as it will break existing imports.
🏁 Script executed:
#!/bin/bash # Search for imports of Translations to identify places that need updating rg "import \{.*Translations.*\} from ['\"].*i18n/Translations['\"]" --type tsLength of output: 83
Breaking Change: Update Import Syntax for Translations Module
The update in
packages/core/src/i18n/Translations.ts(Line 80) now exports theTranslationsclass as a default export. This means that all imports need to be adjusted from the previous named-import syntax:
- Old:
import { Translations } from '.../i18n/Translations'- New:
import Translations from '.../i18n/Translations'Please ensure that:
- All legacy named imports have been updated accordingly.
- The migration guide includes clear documentation about this breaking change.
Note: An initial automated search did not yield any legacy named imports, but please manually verify across the codebase to confirm that no such usages remain.
🧰 Tools
🪛 Biome (1.9.4)
[error] 80-381: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
Previously, the i18n built-in mechanism managed by the
Translationsclass was always in use.It is now possible to plug custom i18n solutions in addition to the built-in implementation.
By default, maxGraph now use an implementation that doesn't provide any translation. This reduces the size of the
applications that don't want to use the built-in implementation.
The bundle size of examples provided in the repository has been reduced by the following values:
Some methods of the
Translationsclass are now more resilient to errors by accepting nullish parameters:BREAKING CHANGES: The built-in
Translationsclass is no longer used by default. To use it, callGlobalConfig.i18n = new TranslationsAsI18n().Notes
Closes #688
Summary by CodeRabbit
Summary by CodeRabbit
Breaking Changes
Translationsclass is no longer used by default; users must instantiateTranslationsAsI18nto utilize it.New Features
Documentation
TranslationsAsI18n.