-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[WIP] Plugin support #4001
[WIP] Plugin support #4001
Conversation
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Bundle Stats — desktop-clientHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset
View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger
Smaller
Unchanged
|
Bundle Stats — loot-coreHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset
View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger
Smaller No assets were smaller Unchanged No assets were unchanged |
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
packages/desktop-client/src/components/ActualPluginsProvider.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-eslint-plugin". (The package "eslint-plugin-eslint-plugin" was not found when loaded as a Node module from the directory "/packages/eslint-plugin-actual".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-eslint-plugin" was referenced from the config file in "packages/eslint-plugin-actual/.eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. WalkthroughThis pull request introduces a comprehensive set of changes focused on enhancing theme management, plugin support, and configuration across multiple packages in the desktop client application. The modifications span several key areas:
The changes aim to create a more modular and extensible theming and plugin system for the desktop client, allowing for easier customization and future expansion of the application's visual and functional capabilities. Possibly related PRs
Suggested reviewers
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 5
🧹 Nitpick comments (21)
packages/plugins-shared/vite.config.mts (2)
8-8
: Consider adding more DTS plugin options for better type generationWhile the basic DTS plugin configuration works, consider adding these options for better TypeScript support:
rollupTypes: true
for better type bundlingskipDiagnostics: false
for stricter type checking- plugins: [dts({ insertTypesEntry: true, include: ['src'] })], + plugins: [dts({ + insertTypesEntry: true, + include: ['src'], + rollupTypes: true, + skipDiagnostics: false + })],
16-17
: Consider adding source maps for better debuggingFor a shared library package, source maps would be valuable for debugging in development environments.
formats: ['es', 'cjs'], fileName: format => `index.${format}.js`, + }, + sourcemap: true,packages/desktop-client/src/style/theme.tsx (2)
59-71
: Performance note: Spread syntax used in reduce accumulatorsWhile collecting plugin themes, repeated usage of spread syntax in “acc = { ...acc, ... }” inside a reduce can have O(n^2) complexity. If plugin lists become large, consider push or assignment-based merges to improve performance.
Below is an example of how you might refactor:
-acc = { - ...acc, - [theme]: { - name: theme, - colors: plugin.getThemeSchema(theme, false), - } -}; +acc[theme] = { + name: theme, + colors: plugin.getThemeSchema(theme, false), +};
72-84
: Same performance note appliesThe same pattern of object accumulator spread is repeated for the “themesDark”. Consider using property assignment to avoid repeated object copies if performance becomes a concern.
packages/desktop-client/src/components/settings/Themes.tsx (2)
60-78
: Performance note on spread with reduce accumulatorsLike in other files, spread syntax on each iteration can be inefficient at scale. Consider direct property assignment to “acc” object to avoid potential performance issues with large plugin arrays.
🧰 Tools
🪛 Biome (1.9.4)
[error] 69-69: Avoid the use of spread (
...
) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce
) because it causes a time complexity ofO(n^2)
.
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
80-98
: Reusing accumulation pattern for 'themesDark'Again, to maintain performance, avoid repeated full object copying for each iteration. Consider a direct assignment pattern.
🧰 Tools
🪛 Biome (1.9.4)
[error] 88-88: Avoid the use of spread (
...
) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce
) because it causes a time complexity ofO(n^2)
.
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
packages/desktop-client/src/components/ThemeSelector.tsx (4)
64-77
: Accumulating plugin icons with repeated spread usageReusing spread syntax in a reduce accumulator can have performance downsides at scale. Consider direct property assignments to avoid O(n^2) complexity.
🧰 Tools
🪛 Biome (1.9.4)
[error] 70-70: Avoid the use of spread (
...
) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce
) because it causes a time complexity ofO(n^2)
.
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
79-92
: Same pattern for 'pluginIconsDark'Again, prefer direct assignments if performance concerns arise. Current approach is correct functionally, but not optimal in large plugin scenarios.
🧰 Tools
🪛 Biome (1.9.4)
[error] 84-84: Avoid the use of spread (
...
) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce
) because it causes a time complexity ofO(n^2)
.
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
93-111
: Collecting plugin-provided themes (light, dark) via reduceSimilar performance reference: repeated object spreads in a loop can degrade performance. Library usage or direct assignments might be more efficient.
🧰 Tools
🪛 Biome (1.9.4)
[error] 101-101: Avoid the use of spread (
...
) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce
) because it causes a time complexity ofO(n^2)
.
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
133-134
: Merging extended plugin themesSame merge pattern as in other components. It’s functionally appropriate while having the same performance caution.
packages/plugins-shared/src/interfaces/actualPluginEntry.ts (1)
1-5
: Well-designed plugin entry point type!The
ActualPluginEntry
type follows a good pattern by injecting React as a parameter, which helps avoid potential React version conflicts between the host application and plugins. This approach ensures that plugins use the same React instance as the host application.Consider documenting this pattern in the plugin development guide to help plugin authors understand why React is passed as a parameter and how to properly structure their plugins.
packages/plugins-shared/src/globals.d.ts (1)
1-6
: Good approach for extending React's CSS types!The extension of React's
CSSProperties
with Emotion'sCSSObject
is a clean way to enhance TypeScript's CSS type checking for your theming system. The ESLint disable comment is appropriately used and documented.Consider adding a comment explaining why this extension is necessary for the theming system, to help future maintainers understand its purpose.
packages/plugins-shared/src/interfaces/actualPlugin.ts (3)
5-8
: Add validation and documentation for plugin metadataThe required
name
andversion
fields lack format validation and documentation. Consider:
- Adding JSDoc comments describing expected formats
- Adding version format validation (e.g., semver)
Example documentation:
/** * Interface defining the contract for Actual Budget plugins */ export interface ActualPlugin { /** Unique identifier for the plugin. Should be kebab-case */ name: string; /** Plugin version following semver (e.g., "1.0.0") */ version: string; // ... }
8-14
: Document theme-related method contractsThe theme-related methods lack documentation about their expected behavior and return values. Add JSDoc comments describing:
- Expected theme name formats
- Icon requirements/constraints
- Schema structure expectations
15-22
: Clean up or document future hooksThe commented-out hooks section suggests incomplete functionality. Either:
- Remove the commented code until ready for implementation, or
- Add TODO comments explaining the planned functionality and timeline
packages/loot-core/src/types/prefs.d.ts (1)
89-89
: Document customTheme relationship with Theme typeThe
customTheme
property lacks documentation explaining:
- Its relationship with the
theme
property- When it's used vs. built-in themes
- Format expectations
packages/desktop-client/src/components/ActualPluginsProvider.tsx (2)
124-151
: Improve plugin loading architectureThe current implementation has several architectural concerns:
- No plugin validation/sandboxing
- Basic error handling
- No retry logic
- No version management
Consider:
- Implementing a plugin sandbox using Web Workers
- Adding proper validation of plugin exports
- Implementing retry logic with backoff
- Adding version compatibility checks
83-122
: Clean up or implement commented GitHub integrationThe commented GitHub integration code should either be:
- Implemented with proper error handling and security measures
- Removed until ready for implementation
- Documented with TODO explaining the implementation plan
packages/plugins-shared/src/interfaces/themeDefinition.ts (1)
1-196
: Add JSDoc documentation and consider type safety improvements.While the interface is well-structured, consider these enhancements:
- Add JSDoc documentation to describe:
- Expected color formats (hex, rgb, etc.)
- Usage guidelines for different theme properties
- Examples of valid values
- Consider using more specific types for color values:
+/** Theme interface for customizing the application's appearance. + * @property pageBackground - Main background color (hex: #RRGGBB or rgb(r,g,b)) + * @property pageText - Main text color + * ... + */ export interface ThemeDefinition { - pageBackground?: string; + pageBackground?: `#${string}` | `rgb(${number},${number},${number})`; // Apply similar type constraints to other color propertiespackages/loot-core/src/server/main.ts (2)
1335-1337
: Add validation for customTheme value.Consider adding validation for the
customTheme
value before saving it to ensure it contains valid theme data.if ('customTheme' in prefs) { + if (typeof prefs.customTheme !== 'string') { + throw new Error('customTheme must be a string'); + } await asyncStorage.setItem('customTheme', prefs.customTheme); }
1383-1384
: Consider extracting theme validation logic.The theme validation logic could be more maintainable if extracted into a separate function. This would make it easier to add new themes and maintain the validation logic in one place.
+ const isValidTheme = (theme: string) => { + const validThemes = ['light', 'dark', 'auto', 'development', 'midnight']; + return validThemes.includes(theme) || Boolean(customTheme); + }; + return { floatingSidebar: floatingSidebar === 'true' ? true : false, maxMonths: stringToInteger(maxMonths || ''), documentDir: documentDir || getDefaultDocumentDir(), keyId: encryptKey && JSON.parse(encryptKey).id, - theme: - theme === 'light' || - theme === 'dark' || - theme === 'auto' || - theme === 'development' || - theme === 'midnight' || - customTheme - ? theme - : 'auto', + theme: isValidTheme(theme) ? theme : 'auto', customTheme, preferredDarkTheme: preferredDarkTheme === 'dark' || preferredDarkTheme === 'midnight'Also applies to: 1387-1387
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/plugins-shared/README.md
is excluded by!**/*.md
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (18)
.eslintrc.js
(1 hunks)packages/desktop-client/public/index.es.js
(1 hunks)packages/desktop-client/src/components/ActualPluginsProvider.tsx
(1 hunks)packages/desktop-client/src/components/App.tsx
(2 hunks)packages/desktop-client/src/components/ThemeSelector.tsx
(2 hunks)packages/desktop-client/src/components/settings/Themes.tsx
(3 hunks)packages/desktop-client/src/style/theme.tsx
(5 hunks)packages/loot-core/src/server/main.ts
(4 hunks)packages/loot-core/src/types/prefs.d.ts
(2 hunks)packages/plugins-shared/.gitignore
(1 hunks)packages/plugins-shared/package.json
(1 hunks)packages/plugins-shared/src/globals.d.ts
(1 hunks)packages/plugins-shared/src/index.ts
(1 hunks)packages/plugins-shared/src/interfaces/actualPlugin.ts
(1 hunks)packages/plugins-shared/src/interfaces/actualPluginEntry.ts
(1 hunks)packages/plugins-shared/src/interfaces/themeDefinition.ts
(1 hunks)packages/plugins-shared/tsconfig.json
(1 hunks)packages/plugins-shared/vite.config.mts
(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- packages/plugins-shared/tsconfig.json
- packages/plugins-shared/.gitignore
- packages/plugins-shared/package.json
🧰 Additional context used
🪛 Biome (1.9.4)
packages/plugins-shared/src/globals.d.ts
[error] 7-8: This empty export is useless because there's another export or import.
This import makes useless the empty export.
Safe fix: Remove this useless empty export.
(lint/complexity/noUselessEmptyExport)
packages/desktop-client/src/components/settings/Themes.tsx
[error] 69-69: Avoid the use of spread (...
) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce
) because it causes a time complexity of O(n^2)
.
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
[error] 88-88: Avoid the use of spread (...
) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce
) because it causes a time complexity of O(n^2)
.
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
packages/desktop-client/src/components/ThemeSelector.tsx
[error] 70-70: Avoid the use of spread (...
) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce
) because it causes a time complexity of O(n^2)
.
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
[error] 84-84: Avoid the use of spread (...
) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce
) because it causes a time complexity of O(n^2)
.
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
[error] 101-101: Avoid the use of spread (...
) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce
) because it causes a time complexity of O(n^2)
.
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
[error] 120-120: Avoid the use of spread (...
) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce
) because it causes a time complexity of O(n^2)
.
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
🔇 Additional comments (41)
packages/plugins-shared/vite.config.mts (2)
1-6
: LGTM: Imports and basic setup look good
The imports and basic configuration setup follow best practices, using proper ES module imports and the recommended Vite configuration pattern.
10-18
: Verify the library name usage across the codebase
The library name 'Shared' is used as a global name. Ensure this doesn't conflict with other globals in the application.
✅ Verification successful
No conflict with the 'Shared' library name
The search results show that 'Shared' is only used as a library name in the Vite configuration and there are no conflicts with global variables. Other occurrences of "Shared" in the codebase are related to the browser's SharedArrayBuffer
API, which is a different context and doesn't conflict with the library name.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any potential naming conflicts with 'Shared'
rg -l "(?:var|let|const|window\.)Shared\b" --type ts --type tsx
Length of output: 93
Script:
#!/bin/bash
# Search for any potential naming conflicts with 'Shared' in all files
rg -l "(?:var|let|const|window\.)Shared\b"
# Also check for any usage of 'Shared' in general to understand the context
rg "Shared" -C 2
Length of output: 6915
packages/desktop-client/src/style/theme.tsx (12)
7-8
: Exporting ThemeDefinition is appropriate
Importing and using “ThemeDefinition” clarifies theme typing across multiple files, promoting type consistency. No issues found here.
16-16
: Good practice: Exporting 'themes' for external access
Exporting “themes” fosters modularity and meaningful cross-component usage. The structure is clean, and the environment-based addition of “development” is well-encapsulated with a condition.
37-38
: Neat exposure of customTheme
Exposing “[customTheme, setCustomTheme]” ensures that custom themes can be loaded and saved dynamically. Looks good.
48-50
: Use of useState with 'themesExtended'
“themesExtended” merges base themes with plugin themes. Straightforward approach, no issues noted here.
52-54
: Maintaining themeColors in state
Storing “themeColors” locally with useState is a sensible approach, ensuring updates propagate correctly. No concerns here.
56-56
: Proper destructuring from useActualPlugins
Neatly extracts “plugins” from “useActualPlugins”, facilitating plugin-driven theme logic. No issues found.
85-86
: Merging plugin-provided themes with base themes
By calling “setThemesExtended({ ...themes, ...themesLight, ...themesDark }),” you incorporate plugin-based themes seamlessly. This approach is correct from a functionality standpoint. Watch out for potential naming collisions if a plugin-based theme’s key matches an existing base theme name.
88-92
: Loading customTheme from global prefs
Properly prioritizes “customTheme” if it exists, returning early after setting theme colors. This is a valid approach for user customizations.
94-101
: Responsive theme switching for 'auto' preference
Effectively handles system dark-mode preference changes at runtime by listening to the media query. Good approach for real-time theme updates.
116-116
: Fallback to the 'light' theme
When the system preference is set to “light,” the code sets “themesExtended[‘light’].colors.” This fallback ensures consistent theming. No issues identified.
126-126
: Applying user-specified theme
When not using “auto,” it directly sets the chosen theme’s colors. Straightforward mechanism, no issues detected.
128-128
: Overall thorough logic
The final effect returns a unified <style> element reflecting the currently active or custom theme. This is logically correct, clean, and modular.
packages/desktop-client/src/components/settings/Themes.tsx (5)
42-47
: Defining ThemesExtendedType
This custom type helps unify plugin-based and base themes under a single shape, improving readability and type safety.
52-52
: Introducing setThemeObject in useTheme array destructuring
• “setThemeObject” usage hints at storing a full theme object into global prefs.
• “themesExtended” and “themeOptionsExtended” facilitate dynamic plugin-based theme insertion.
All changes look consistent with the new plugin-driven theming approach.
Also applies to: 54-59
99-100
: Merging plugin themes with base 'themes'
Appropriately merges plugin-based light/dark themes into a single extended structure. Watch for name collisions if a plugin uses a name that is already defined in “themes.”
102-108
: Updating themeOptionsExtended from themesExtended
Efficient approach to keep the UI’s selection options in sync with loaded plugin themes. Looks good.
131-136
: Conditional call to setThemeObject
When a user selects a theme not originally in “themes,” it attempts to store that as a custom theme via JSON serialization. The logic is straightforward. Ensure robust JSON error handling if malformed data is encountered.
packages/desktop-client/src/components/ThemeSelector.tsx (9)
1-8
: Importing new hooks and types
Imports for “useEffect,” “useMemo,” “SVGProps,” etc., reflect the expanded dynamic theming. No issues noted on these lines.
13-13
: Adding ThemeDefinition, themes, useTheme, useActualPlugins
These additions are consistent with the plugin-based theming approach across the codebase. Implementation aligns with usage in other modules.
Also applies to: 15-15, 17-17
21-21
: Setup for ThemesExtendedType, ThemesIconsType, and local states
• “themesExtended” ensures that plugin-provided themes can be appended.
• “themeIcons” allows distinct icons per plugin-based theme.
Well-structured approach.
Also applies to: 28-37
43-46
: useState initialization with base 'themes' and 'themeOptions'
Establishes default states before layering custom plugin logic. This is consistent with the broader theming mechanism.
51-60
: Memoization of baseIcons
The “useMemo” approach for base theme icons prevents unnecessary re-renders. Implementation is correct.
61-62
: Stateful 'themeIcons'
Retains plugin-injected icons for new themes. No immediate concerns, usage is consistent with the final merges.
131-132
: Combining baseIcons with pluginIcons
Merging “baseIcons” with “pluginIconsLight” and “pluginIconsDark” keeps the logic straightforward. Again, watch out for key collisions.
136-142
: Keep themeOptionsExtended in sync
Syncing “themeOptionsExtended” to newly extended themes is consistent with the dynamic plugin approach across the codebase. No issues detected.
175-175
: Rendering theme menu items with extended options
Now referencing the merged “themeOptionsExtended” ensures plugin-based themes are fully integrated. Implementation is correct.
packages/desktop-client/public/index.es.js (7)
1-17
: Dynamic SVG creation via 'c' and usage in 'u'
• Using a factory function to create SVG with inherited style is a neat approach.
• Proper usage of “...t” spread for props is acceptable here since it’s not in a tight loop.
No issues found.
18-26
: Return different elements based on themeName
The switch-case toggles between “dracula” and a fallback
28-229
: Dracula theme definitions
A thorough, consistent set of color keys for the “Dracula” theme. This approach is consistent with the code’s theming pattern, no functional concerns.
230-412
: Sepia theme definitions
Again, a large mapping of color values for a “Sepia” style theme. The naming convention and consistency appear good.
413-637
: Rose Pine theme definitions
Thorough coloring for each UI element. Similar approach with clear naming. No correctness issues noted.
638-808
: Tokyo Night theme definitions
Completes the theme set with a large set of color properties. Methodical structure, no logical flaws detected.
809-834
: Default export function 'i' integrating plugin interface
• Exposes “availableThemes,” “getThemeIcon,” and “getThemeSchema” for custom usage.
• The “switch” logic in “d” reads cleanly, returning the correct theme object.
No concerns found.
packages/plugins-shared/src/index.ts (1)
1-3
: Clean type exports forming a well-structured plugin API!
The type-only exports create a clear public API for the plugins package. The separation of interfaces into distinct files while aggregating exports in index.ts follows good TypeScript practices.
packages/plugins-shared/src/globals.d.ts (1)
8-8
: Verify if the empty export is required
While static analysis suggests removing this empty export, it might be necessary to maintain the file as a module. Let's verify its usage across the codebase.
✅ Verification successful
Let me check the actual content of the file that imports from globals to understand its usage.
Empty export is required - it marks the file as a module
The empty export is necessary in this case as the file contains a module augmentation for React's CSSProperties
interface using the declare module
syntax. The empty export ensures the file is treated as a module rather than an ambient declaration file, which is required for module augmentations to work correctly in TypeScript.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if there are any direct imports of this module
rg -l "from ['\"].*globals['\"]"
# Check if there are any references to the CSS properties extension
ast-grep --pattern 'interface $_ extends CSSProperties'
Length of output: 141
Script:
#!/bin/bash
# Check the content of the file importing globals
cat packages/loot-core/src/server/budget/util.ts
# Also check the content of globals.d.ts itself to understand what it declares
cat packages/plugins-shared/src/globals.d.ts
Length of output: 901
🧰 Tools
🪛 Biome (1.9.4)
[error] 7-8: This empty export is useless because there's another export or import.
This import makes useless the empty export.
Safe fix: Remove this useless empty export.
(lint/complexity/noUselessEmptyExport)
packages/desktop-client/src/components/App.tsx (2)
176-218
: LGTM: Provider hierarchy is well-structured.
The ActualPluginsProvider
is correctly positioned as the outermost provider (after BrowserRouter
), ensuring plugin functionality is available throughout the component tree. The existing providers are properly nested within it, maintaining their original order and relationships.
184-189
: LGTM: Theme integration is properly implemented.
The View component's data-theme attribute and style properties are correctly configured for theme support:
- data-theme binding to the theme value
- Proper flex layout properties
- Consistent style structure
.eslintrc.js (1)
66-66
: Verify the scope of ignored files.
The pattern **/public/*.js
will exclude all JavaScript files in any public
directory. Let's verify this won't exclude files that should be linted.
✅ Verification successful
ESLint ignore pattern is appropriate for generated files
The ignore pattern **/public/*.js
is correctly scoped:
- Only affects one public directory:
packages/desktop-client/public/
- The JS file in this directory (
index.es.js
) is clearly a generated file containing bundled SVG components - This is supported by the webpack configuration presence, indicating this is a build output directory
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check what files would be ignored by the new pattern
# List all .js files in public directories
fd -g '**/public/*.js'
# Show the content of these files to verify they are indeed generated/build files
fd -g '**/public/*.js' -x head -n 5
Length of output: 62
Script:
#!/bin/bash
# Let's try a different approach to understand the codebase structure
# and potential impact of the ignore pattern
# First, let's see if there are any public directories
fd -t d 'public'
# Let's also check for any JavaScript files that might be affected
# by looking at the overall JS file structure
fd -e js
# Additionally, let's check if there's any documentation about
# the public directory usage in README or similar files
fd -g 'README*' -X grep -l 'public'
Length of output: 3023
Script:
#!/bin/bash
# Let's examine the content of the only JS file found in public directory
# to verify if it's a generated/build file
head -n 10 "packages/desktop-client/public/index.es.js"
# Let's also check if there are any build configuration files
# that might indicate the purpose of the public directory
fd -g '*webpack*' -X grep -l 'public'
Length of output: 433
packages/loot-core/src/server/main.ts (1)
1360-1360
: LGTM!
The addition of customTheme
to the preferences loading logic is implemented correctly and follows the established pattern.
Also applies to: 1369-1369
external: ['react', 'react-dom', '@emotion/css'], | ||
output: { | ||
globals: { | ||
react: 'React', | ||
'react-dom': 'ReactDOM', | ||
}, | ||
}, |
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.
💡 Codebase verification
🛠️ Refactor suggestion
@emotion/css should be added to globals configuration
Based on the extensive usage of @emotion/css
throughout the codebase (over 50 files importing it) and its current inclusion in the external
array, it should also be configured in the globals
configuration to ensure proper bundling.
external: ['react', 'react-dom', '@emotion/css'],
output: {
globals: {
react: 'React',
'react-dom': 'ReactDOM',
+ '@emotion/css': 'emotionCss',
},
},
🔗 Analysis chain
Consider expanding external dependencies and globals configuration
The current configuration might be incomplete:
@emotion/css
is marked as external but missing in globals- Other potential shared dependencies might need to be added
Consider updating the configuration:
external: ['react', 'react-dom', '@emotion/css'],
output: {
globals: {
react: 'React',
'react-dom': 'ReactDOM',
+ '@emotion/css': 'emotionCss',
},
},
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for other potential shared dependencies in the project
echo "Checking package.json files for common dependencies..."
fd package.json$ --type f --exec jq -r '.dependencies // {} | keys[]' {} \; | sort | uniq -c | sort -nr
# Check for @emotion/css usage
echo "Checking @emotion/css usage patterns..."
rg --type ts --type tsx "from ['|\"]@emotion/css['|\"]" -A 3
Length of output: 4850
Script:
#!/bin/bash
# Let's fix the previous commands and try again
# Check package dependencies
echo "Checking package.json files for dependencies..."
fd package.json --type f --exec cat {} \; | jq -r '.dependencies | keys[]' 2>/dev/null | sort | uniq -c | sort -nr
# Check for @emotion/css imports
echo -e "\nChecking @emotion/css usage patterns..."
rg "from ['\"]@emotion/css['\"]"
# Check for any other emotion-related imports
echo -e "\nChecking other emotion-related imports..."
rg "from ['\"]@emotion/"
Length of output: 13786
export type Theme = | ||
| 'light' | ||
| 'dark' | ||
| 'auto' | ||
| 'midnight' | ||
| 'development' | ||
| string; |
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.
🛠️ Refactor suggestion
Consider using a branded type for Theme safety
Adding string
to the Theme union type reduces type safety. Consider:
- Using a branded type to ensure theme names are valid
- Creating a theme registry type for plugin-provided themes
Example:
type BuiltInTheme = 'light' | 'dark' | 'auto' | 'midnight' | 'development';
type BrandedPluginTheme = string & { __brand: 'PluginTheme' };
export type Theme = BuiltInTheme | BrandedPluginTheme;
const loaded = await Promise.all([ | ||
loadPluginFromRepo('https://github.com/actual-plugins/example'), | ||
]); |
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.
Security: Implement plugin source validation
Loading plugins directly from GitHub without validation poses security risks. Consider:
- Implementing a plugin registry/marketplace
- Adding plugin signature verification
- Using a whitelist of approved plugin sources
packages/desktop-client/src/components/ActualPluginsProvider.tsx
Outdated
Show resolved
Hide resolved
packages/desktop-client/src/components/ActualPluginsProvider.tsx
Outdated
Show resolved
Hide resolved
👋 Hi! It looks like this PR has not had any changes for a week now. Would you like someone to review this PR? If so - please remove the "[WIP]" prefix from the PR title. That will let the community know that this PR is open for a review. |
@coderabbitai review |
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: 7
♻️ Duplicate comments (2)
packages/desktop-client/src/components/ActualPluginsProvider.tsx (2)
34-36
:⚠️ Potential issueSecurity: Implement plugin source validation and configuration
Loading plugins from a hardcoded GitHub repository URL is not production-ready and poses security risks.
Consider:
- Moving the plugin source URL to a configuration file
- Implementing plugin source validation
- Adding support for multiple plugin sources
- Using HTTPS endpoints only
- const loaded = await Promise.all([ - loadPluginFromRepo('https://github.com/actual-plugins/example'), - ]); + const pluginSources = await getConfiguredPluginSources(); + const loaded = await Promise.all( + pluginSources.map(source => loadPluginFromRepo(source)) + );
67-67
:⚠️ Potential issueRemove global React assignment
Setting
window.React
is an anti-pattern that creates global namespace pollution and may cause version conflicts.Consider passing React as a parameter through the plugin API instead.
🧹 Nitpick comments (10)
packages/plugins-shared/src/interfaces/actualPluginManifest.ts (1)
1-8
: Consider using enums or documented constant types forpluginType
.
While this interface is clear and concise, using anenum
or well-documented constant union type forpluginType
could enhance its extensibility and maintainability if more plugin types are introduced later.packages/plugins-shared/package.json (1)
1-21
: Consider adding missing metadata fields and aligning dependency versioning strategies.
Adding fields such aslicense
,repository
, orauthor
can make the package more informative. Also, it might be beneficial to standardize how versions are pinned (e.g., caret vs. exact) across dependencies for consistency.packages/desktop-client/src/style/theme.tsx (3)
16-16
: Consider documenting exportedthemes
.
Thethemes
object is instrumental to theming. Adding a short JSDoc or comment explaining each built-in theme’s purpose can improve clarity for future maintainers.
58-73
: Consider prefixing plugin-provided themes to avoid name collisions.
If different plugins define a “dark” or “light” theme, merging them here might cause a collision. Prefixing with the plugin’s slug or name could provide uniqueness.
Line range hint
82-115
: Refactor repetitive assignment of theme colors.
Multiple lines assignthemeColors
from eitherdarkTheme.colors
orthemesExtended['light'].colors
. Extracting a small helper function can make it more readable and reduce duplication.packages/desktop-client/src/components/ActualPluginsProvider.tsx (1)
83-122
: Remove or implement commented codeThe commented-out utility functions for GitHub integration appear to be well-structured but are currently unused.
Either:
- Remove the commented code if it's not needed
- Implement the functionality if it's required
- Add a TODO comment explaining why it's preserved
packages/desktop-client/src/components/ThemeSelector.tsx (1)
64-71
: Simplify plugin theme aggregationThe plugin theme aggregation logic is duplicated and could be simplified.
Consider extracting the common validation logic into a utility function:
const validatePlugin = (plugin: ActualPlugin) => plugin?.availableThemes?.() ?? []; const getValidThemes = (plugin: ActualPlugin) => validatePlugin(plugin).filter(theme => theme !== undefined);Also applies to: 83-93
🧰 Tools
🪛 Biome (1.9.4)
[error] 68-69: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/desktop-client/src/components/settings/Themes.tsx (1)
116-123
: Improve theme object handlingThe theme object handling could be more robust.
Consider:
- Using type guards for theme validation
- Adding error handling for invalid theme objects
- Implementing a more explicit state management approach
- if ( - !Object.keys(themes).some(t => t === value) && - typeof themesExtended === 'object' - ) { - setThemeObject(JSON.stringify(themesExtended?.[value] ?? '')); - } else { - setThemeObject(null); - } + const themeObject = getThemeObject(value, themes, themesExtended); + setThemeObject(themeObject);packages/desktop-client/public/index.es.js (2)
1-19
: Enhance SVG icon accessibility and flexibility.The SVG icon implementation could be improved in the following areas:
- Add ARIA attributes for accessibility
- Make the icon path configurable instead of hardcoded
- Make the viewBox dimensions configurable
Consider applying these changes:
const c = (e) => (a) => /* @__PURE__ */ e.createElement( "svg", { ...a, xmlns: "http://www.w3.org/2000/svg", - viewBox: "0 0 20 20", + viewBox: a.viewBox || "0 0 20 20", + role: "img", + "aria-hidden": "true", style: { color: "inherit", ...a.style } }, /* @__PURE__ */ e.createElement( "path", { - d: "M11 9.27V0l6 11-4 6H7l-4-6L9 0v9.27a2 2 0 1 0 2 0zM6 18h8v2H6v-2z", + d: a.path || "M11 9.27V0l6 11-4 6H7l-4-6L9 0v9.27a2 2 0 1 0 2 0zM6 18h8v2H6v-2z", fill: "currentColor" } ) )
28-804
: Consider enhancing theme configuration maintainability.The theme configurations are well-structured and thoroughly documented. However, consider these improvements:
- Extract common color values into shared variables
- Use CSS variables for runtime theme customization
- Add color contrast validation for accessibility
Would you like me to help generate:
- A color palette extraction utility
- CSS variable integration
- A color contrast validation script?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
upcoming-release-notes/4001.md
is excluded by!**/*.md
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (10)
packages/desktop-client/public/index.es.js
(1 hunks)packages/desktop-client/src/components/ActualPluginsProvider.tsx
(1 hunks)packages/desktop-client/src/components/ThemeSelector.tsx
(2 hunks)packages/desktop-client/src/components/settings/Themes.tsx
(3 hunks)packages/desktop-client/src/style/theme.tsx
(5 hunks)packages/loot-core/src/types/prefs.d.ts
(2 hunks)packages/plugins-shared/package.json
(1 hunks)packages/plugins-shared/src/index.ts
(1 hunks)packages/plugins-shared/src/interfaces/actualPlugin.ts
(1 hunks)packages/plugins-shared/src/interfaces/actualPluginManifest.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/loot-core/src/types/prefs.d.ts
- packages/plugins-shared/src/interfaces/actualPlugin.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/desktop-client/src/components/ThemeSelector.tsx
[error] 68-69: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 86-87: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 74-74: Avoid the use of spread (...
) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce
) because it causes a time complexity of O(n^2)
.
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
[error] 95-95: Avoid the use of spread (...
) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce
) because it causes a time complexity of O(n^2)
.
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
packages/desktop-client/src/components/settings/Themes.tsx
[error] 64-65: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 73-73: Avoid the use of spread (...
) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce
) because it causes a time complexity of O(n^2)
.
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
🔇 Additional comments (8)
packages/plugins-shared/src/index.ts (1)
1-5
: Export statements look good.
Re-exporting each type individually keeps the index file clean and explicit.
packages/desktop-client/src/style/theme.tsx (6)
7-8
: Imports from plugins-shared and ActualPluginsProvider look correct.
These imports properly integrate plugin-related types and logic into the theme system.
37-38
: Double-check the structure of customTheme
.
If customTheme
is expected to be valid JSON, handling parse failures (e.g., via a try/catch in the consumer) could prevent runtime errors in case the stored preference is corrupted.
48-50
: State setup is straightforward.
Initializing themesExtended
from the default themes
simplifies the subsequent merging logic.
52-55
: Good approach using a union type for themeColors
.
Managing themeColors
as ThemeDefinition | undefined
is a clean way to handle the initialization cycle until themes are resolved.
56-57
: Properly obtaining loadedPlugins
.
Retrieving plugins from the context ensures the plugin-based themes can be integrated cleanly.
80-80
: Blank line is fine.
No special concerns.
packages/desktop-client/src/components/ActualPluginsProvider.tsx (1)
137-137
:
Security: Remove hardcoded plugin path
Using a hardcoded path '/index.es.js' is not production-ready and poses security risks.
Consider:
- Implementing a proper plugin registry
- Using versioned plugin URLs
- Adding integrity checks for downloaded content
- const response = await fetch('/index.es.js');
+ const pluginMetadata = await fetchPluginMetadata(repo);
+ const response = await fetch(pluginMetadata.url, {
+ headers: { 'Accept': 'application/javascript' }
+ });
Likely invalid or redundant comment.
if (customTheme) { | ||
setThemeColors(JSON.parse(customTheme).colors); | ||
return; | ||
} |
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.
🛠️ Refactor suggestion
Handle malformed customTheme
parsing.
JSON parsing errors can break the component. Consider a safeguard like try/catch or a fallback if parsing fails.
- setThemeColors(JSON.parse(customTheme).colors);
+ try {
+ const parsed = JSON.parse(customTheme);
+ setThemeColors(parsed.colors);
+ } catch (e) {
+ console.error('Invalid customTheme JSON', e);
+ setThemeColors(undefined);
+ }
📝 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.
if (customTheme) { | |
setThemeColors(JSON.parse(customTheme).colors); | |
return; | |
} | |
if (customTheme) { | |
try { | |
const parsed = JSON.parse(customTheme); | |
setThemeColors(parsed.colors); | |
} catch (e) { | |
console.error('Invalid customTheme JSON', e); | |
setThemeColors(undefined); | |
} | |
return; | |
} |
const indexJsBlob = await response.blob(); | ||
|
||
console.log(`Plugin “${repo}” loaded successfully.`); | ||
return await loadPluginScript(indexJsBlob); |
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.
🛠️ Refactor suggestion
Enhance plugin loading error handling
The current implementation logs success before actually loading the plugin and provides minimal error context.
Consider:
- Moving the success log after successful plugin initialization
- Adding detailed error information
- Implementing plugin validation before loading
- console.log(`Plugin "${repo}" loaded successfully.`);
- return await loadPluginScript(indexJsBlob);
+ const plugin = await loadPluginScript(indexJsBlob);
+ await validatePlugin(plugin);
+ console.log(`Plugin "${repo}" v${plugin.version} loaded successfully.`);
+ return plugin;
Committable suggestion skipped: line range outside the PR's diff.
...acc, | ||
[theme]: (props: SVGProps<SVGSVGElement>) => | ||
plugin?.getThemeIcon?.(theme, props.style) ?? <View />, | ||
}; |
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.
🛠️ Refactor suggestion
Optimize reducer performance
Using spread operator in reducers can lead to O(n²) time complexity.
Consider using array methods or object manipulation:
- acc = {
- ...acc,
- [theme]: (props: SVGProps<SVGSVGElement>) =>
- plugin?.getThemeIcon?.(theme, props.style) ?? <View />,
- };
+ acc[theme] = (props: SVGProps<SVGSVGElement>) =>
+ plugin?.getThemeIcon?.(theme, props.style) ?? <View />;
📝 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.
...acc, | |
[theme]: (props: SVGProps<SVGSVGElement>) => | |
plugin?.getThemeIcon?.(theme, props.style) ?? <View />, | |
}; | |
acc[theme] = (props: SVGProps<SVGSVGElement>) => | |
plugin?.getThemeIcon?.(theme, props.style) ?? <View />; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 74-74: Avoid the use of spread (...
) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce
) because it causes a time complexity of O(n^2)
.
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
acc = { | ||
...acc, | ||
[theme]: { | ||
name: theme, | ||
colors: plugin?.getThemeSchema?.(theme) ?? {}, | ||
}, | ||
}; | ||
}); |
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.
🛠️ Refactor suggestion
Optimize reducer performance and extract shared logic
This component has similar performance issues and duplicated logic as ThemeSelector.
- Extract shared theme management logic into a custom hook
- Optimize reducer performance by avoiding spread operator
- Consider implementing a theme registry service
// themeRegistry.ts
export const useThemeRegistry = () => {
const [themes, setThemes] = useState({});
const registerTheme = (theme: string, definition: ThemeDefinition) => {
setThemes(current => {
current[theme] = definition;
return current;
});
};
return { themes, registerTheme };
};
🧰 Tools
🪛 Biome (1.9.4)
[error] 73-73: Avoid the use of spread (...
) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce
) because it causes a time complexity of O(n^2)
.
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
), u = (e) => { | ||
const t = c(e); | ||
return ({ themeName: d, darkMode: r, style: o }) => { | ||
switch (d) { | ||
case "dracula": | ||
return /* @__PURE__ */ e.createElement(t, { style: o }); | ||
} | ||
return /* @__PURE__ */ e.createElement("div", { style: o }); | ||
}; | ||
}, b = { |
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.
Fix theme name consistency and improve fallback handling.
The theme icon component has several issues:
- Theme name case mismatch: "dracula" vs "Dracula" in
availableThemes
- Empty div fallback provides poor UX for unsupported themes
- No error handling for invalid theme names
Consider applying these changes:
const u = (e) => {
const t = c(e);
return ({ themeName: d, darkMode: r, style: o }) => {
+ const normalizedTheme = d?.toLowerCase();
switch (d) {
- case "dracula":
+ case "dracula": // TODO: Remove after migration
+ case "Dracula":
return /* @__PURE__ */ e.createElement(t, { style: o });
+ default:
+ console.warn(`Theme icon not found for theme: ${d}`);
+ return /* @__PURE__ */ e.createElement(t, {
+ style: { ...o, opacity: 0.5 },
+ title: `${d} theme`
+ });
}
- return /* @__PURE__ */ e.createElement("div", { style: o });
};
};
📝 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.
), u = (e) => { | |
const t = c(e); | |
return ({ themeName: d, darkMode: r, style: o }) => { | |
switch (d) { | |
case "dracula": | |
return /* @__PURE__ */ e.createElement(t, { style: o }); | |
} | |
return /* @__PURE__ */ e.createElement("div", { style: o }); | |
}; | |
}, b = { | |
), u = (e) => { | |
const t = c(e); | |
return ({ themeName: d, darkMode: r, style: o }) => { | |
const normalizedTheme = d?.toLowerCase(); | |
switch (d) { | |
case "dracula": // TODO: Remove after migration | |
case "Dracula": | |
return /* @__PURE__ */ e.createElement(t, { style: o }); | |
default: | |
console.warn(`Theme icon not found for theme: ${d}`); | |
return /* @__PURE__ */ e.createElement(t, { | |
style: { ...o, opacity: 0.5 }, | |
title: `${d} theme` | |
}); | |
} | |
}; | |
}, b = { |
}, i = (e) => { | ||
const t = u(e), a = { | ||
name: "Example", | ||
version: "0.0.1", | ||
availableThemes: () => ["Dracula", "Sepia", "Rose Pine", "Tokyo Night"], | ||
getThemeIcon: (r, o, n) => /* @__PURE__ */ e.createElement(t, { themeName: r, darkMode: o, style: n }), | ||
getThemeSchema: d | ||
}; | ||
function d(r) { | ||
switch (r) { | ||
case "Dracula": | ||
return b; | ||
case "Rose Pine": | ||
return g; | ||
case "Tokyo Night": | ||
return f; | ||
case "Sepia": | ||
return l; | ||
} | ||
return {}; | ||
} | ||
return a; | ||
}; | ||
export { | ||
i as default | ||
}; |
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.
🛠️ Refactor suggestion
Improve theme management robustness and maintainability.
The theme management implementation could be enhanced:
- Version should be sourced from package.json
- Add theme existence validation
- Replace switch with object lookup
- Add TypeScript types
Consider applying these changes:
+const THEMES = {
+ "Dracula": b,
+ "Rose Pine": g,
+ "Tokyo Night": f,
+ "Sepia": l,
+};
const i = (e) => {
const t = u(e), a = {
name: "Example",
- version: "0.0.1",
+ version: process.env.PACKAGE_VERSION || "0.0.1",
availableThemes: () => ["Dracula", "Sepia", "Rose Pine", "Tokyo Night"],
getThemeIcon: (r, o, n) => /* @__PURE__ */ e.createElement(t, { themeName: r, darkMode: o, style: n }),
getThemeSchema: d
};
function d(r) {
- switch (r) {
- case "Dracula":
- return b;
- case "Rose Pine":
- return g;
- case "Tokyo Night":
- return f;
- case "Sepia":
- return l;
- }
- return {};
+ if (!r || !THEMES[r]) {
+ console.warn(`Theme not found: ${r}`);
+ return {};
+ }
+ return THEMES[r];
}
return a;
};
📝 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.
}, i = (e) => { | |
const t = u(e), a = { | |
name: "Example", | |
version: "0.0.1", | |
availableThemes: () => ["Dracula", "Sepia", "Rose Pine", "Tokyo Night"], | |
getThemeIcon: (r, o, n) => /* @__PURE__ */ e.createElement(t, { themeName: r, darkMode: o, style: n }), | |
getThemeSchema: d | |
}; | |
function d(r) { | |
switch (r) { | |
case "Dracula": | |
return b; | |
case "Rose Pine": | |
return g; | |
case "Tokyo Night": | |
return f; | |
case "Sepia": | |
return l; | |
} | |
return {}; | |
} | |
return a; | |
}; | |
export { | |
i as default | |
}; | |
const THEMES = { | |
"Dracula": b, | |
"Rose Pine": g, | |
"Tokyo Night": f, | |
"Sepia": l, | |
}; | |
const i = (e) => { | |
const t = u(e), a = { | |
name: "Example", | |
version: process.env.PACKAGE_VERSION || "0.0.1", | |
availableThemes: () => ["Dracula", "Sepia", "Rose Pine", "Tokyo Night"], | |
getThemeIcon: (r, o, n) => /* @__PURE__ */ e.createElement(t, { themeName: r, darkMode: o, style: n }), | |
getThemeSchema: d | |
}; | |
function d(r) { | |
if (!r || !THEMES[r]) { | |
console.warn(`Theme not found: ${r}`); | |
return {}; | |
} | |
return THEMES[r]; | |
} | |
return a; | |
}; | |
export { | |
i as default | |
}; |
👋 Hi! It looks like this PR has not had any changes for a week now. Would you like someone to review this PR? If so - please remove the "[WIP]" prefix from the PR title. That will let the community know that this PR is open for a review. |
This PR was closed because it has been stalled for 5 days with no activity. |
Second implementation of plugin support. This is not ready