feat(web): plugin playground/reflect code editor to viewer #1224
Conversation
WalkthroughThe changes introduce a new React functional component in Changes
Possibly related PRs
Suggested reviewers
Poem
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
Documentation and Community
|
✅ Deploy Preview for reearth-web ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
…eearth/reearth-visualizer into playground/reflect-code-to-visual
devshun
changed the title
…eearth/reearth-visualizer into playground/reflect-code-to-visual
|
thanks for your review!
This branch was branched off from #1216, so I changed the base branch to make the changes (files changed) easier to track. After merging #1216, I plan to merge this branch into main🙏 |
…eearth/reearth-visualizer into playground/reflect-code-to-visual
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## playground/display-code-editor #1224 +/- ##
=================================================================
Coverage ? 18.24%
=================================================================
Files ? 619
Lines ? 67423
Branches ? 622
=================================================================
Hits ? 12302
Misses ? 55121
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Thanks @devshun ! Changes look good. Since this PR is not pointing to main, it can be directly merged into the target branch without an approval I think. Sorry, I didn't notice this earlier. |
devshun
dismissed
mulengawilfred’s stale review
The base branch was changed.
…into playground/reflect-code-to-visual
…into playground/reflect-code-to-visual
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (9)
web/src/beta/features/PluginPlayground/Plugins/constants.ts (1)
18-31: LGTM! Consider enhancing maintainability.The YAML structure is well-organized with clear separation of plugin metadata and extension configuration. However, consider these improvements:
- Use template literals with proper YAML indentation for better readability
- Add comments explaining the expected schema
- Consider a more descriptive widget description
Here's a suggested improvement:
sourceCode: `id: demo-widget name: Test plugin version: 1.0.0 +# Plugin extensions define the components and their configurations extensions: - id: demo-widget type: widget name: Demo Widget - description: Demo widget + description: A demo widget showcasing plugin reflection capabilities widgetLayout: defaultLocation: zone: outer section: left area: top `,web/src/beta/features/PluginPlayground/Code/index.tsx (2)
34-34: Consider adding execution feedback statesThe execute button could benefit from visual feedback during code execution. Consider:
- Adding a loading state while code is executing
- Providing visual feedback for successful/failed execution
- Disabling the button during execution
- <Button icon="playRight" iconButton onClick={executeCode} /> + <Button + icon={isExecuting ? "loading" : "playRight"} + iconButton + onClick={executeCode} + disabled={isExecuting} + />
37-41: Consider debouncing source code changesFor better performance, consider debouncing the
onChangeSourceCodecallback to prevent excessive updates when typing rapidly.+import { useCallback } from "react"; +import debounce from "lodash/debounce"; const Code: FC<Props> = ({ ... }) => { + const debouncedOnChange = useCallback( + debounce((value: string | undefined) => onChangeSourceCode(value), 300), + [onChangeSourceCode] + ); + return ( ... <CodeInput language={getLanguageByFileExtension(fileTitle)} value={sourceCode} - onChange={onChangeSourceCode} + onChange={debouncedOnChange} />web/src/beta/features/PluginPlayground/hooks.tsx (2)
97-103: Consider optimizing the source code update handlerWhile the implementation is functional, consider these improvements:
- The callback could be simplified
- Consider debouncing source code updates to prevent excessive updates during typing
Here's a suggested improvement:
- executeCode={executeCode} - onChangeSourceCode={(value) => { - updateFileSourceCode( - value ?? selectedFile.sourceCode, - selectedFile.id - ); - }} + executeCode={executeCode} + onChangeSourceCode={useCallback( + (value) => updateFileSourceCode(value ?? selectedFile.sourceCode, selectedFile.id), + [selectedFile.id, selectedFile.sourceCode, updateFileSourceCode] + )}Consider adding debouncing:
const debouncedUpdateSourceCode = useMemo( () => debounce(updateFileSourceCode, 300), [updateFileSourceCode] );
115-115: Consider consistent dependency orderingFor better maintainability, consider grouping related dependencies together.
- [selectedFile, handlePluginDownload, executeCode, updateFileSourceCode] + [selectedFile, executeCode, updateFileSourceCode, handlePluginDownload]web/src/beta/features/PluginPlayground/Plugins/hook.ts (2)
15-41: Consider improving the demo widget implementation.The demo widget implementation could be enhanced in several ways:
- Move the demo widget template to a separate constants file for better maintainability
- Consider using CSS-in-JS or a separate stylesheet instead of inline styles
- Clean up unnecessary whitespace in the template literal
Here's a suggested refactor:
+ // In constants.ts + export const DEMO_WIDGET_SOURCE = `reearth.ui.show(\` + <style> + body { margin: 0; } + #wrapper { + background: #232226; + height: 100%; + color: white; + border: 3px dotted red; + border-radius: 5px; + padding: 20px 0; + } + </style> + <div id="wrapper"> + <h2 style="text-align: center; margin: 0;">Hello World</h2> + </div> + \`);`; - sourceCode: `reearth.ui.show(\` - <style> - body { - margin: 0; - } - #wrapper { - background: #232226; - height: 100%; - color: white; - border: 3px dotted red; - border-radius: 5px; - padding: 20px 0; - } - </style> - <div id="wrapper"> - <h2 style="text-align: center; margin: 0;">Hello World</h2> - </div> - \`); - - ` + sourceCode: DEMO_WIDGET_SOURCE
131-147: Consider adding error handling for invalid file IDs.The implementation looks good and follows the established patterns in the codebase. However, consider adding error handling for cases where the file ID doesn't exist.
Here's a suggested enhancement:
const updateFileSourceCode = useCallback( (sourceCode: string, id: string) => { + const fileExists = selectedPlugin.files.some(file => file.id === id); + if (!fileExists) { + setNotification({ + type: "error", + text: "Cannot update source code: File not found" + }); + return; + } setPlugins((plugins) => plugins.map((plugin) => plugin.id === selectedPlugin.id ? { ...plugin, files: plugin.files.map((file) => file.id === id ? { ...file, sourceCode } : file ) } : plugin ) ); }, - [selectedPlugin] + [selectedPlugin, setNotification] );web/src/beta/features/PluginPlayground/Code/hook.ts (2)
51-51: Name the exported functional component for better debugging and clarity.Naming the exported functional component can improve stack traces and enhance code readability.
Apply this diff to name the component:
-export default ({ files }: Props) => { +export default function PluginExecutor({ files }: Props) {
93-95: Handle the case where the JavaScript file corresponding to the extension is not found.When the corresponding JavaScript file for an extension is not found, it might be helpful to notify the user or log a warning. This improves debuggability and user feedback.
Consider adding a notification:
if (!file) { + setNotification({ + type: "warning", + text: `JavaScript file for extension "${cur.id}" not found.`, + }); return prv; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (6)
web/src/beta/features/PluginPlayground/Code/hook.ts(1 hunks)web/src/beta/features/PluginPlayground/Code/index.tsx(2 hunks)web/src/beta/features/PluginPlayground/Plugins/constants.ts(1 hunks)web/src/beta/features/PluginPlayground/Plugins/hook.ts(3 hunks)web/src/beta/features/PluginPlayground/Viewer/index.tsx(2 hunks)web/src/beta/features/PluginPlayground/hooks.tsx(4 hunks)
🧰 Additional context used
🪛 GitHub Check: ci-web / ci
web/src/beta/features/PluginPlayground/Code/hook.ts
[failure] 60-60:
'outputs' is assigned a value but never used. Allowed unused vars must match /^_/u
🪛 Biome
web/src/beta/features/PluginPlayground/Code/hook.ts
[error] 108-108: 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 (9)
web/src/beta/features/PluginPlayground/Viewer/index.tsx (3)
6-8: LGTM! Well-structured type definition.
The Props type is well-defined using ComponentProps utility type, ensuring type safety by properly extracting the widgets type from the Visualizer component.
10-10: LGTM! Component signature follows React best practices.
The functional component signature is properly typed with FC and correctly destructures the widgets prop.
29-29: Verify widget integration with Visualizer component.
The widgets prop is correctly passed to the Visualizer component. However, let's verify the widget integration implementation.
web/src/beta/features/PluginPlayground/Plugins/constants.ts (1)
26-30: Verify widget layout configuration compatibility.
The widget layout configuration specifies positioning in the outer zone's left section. Let's verify this is a valid configuration and that all specified zones/sections are supported.
✅ Verification successful
Widget layout configuration is compatible.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for widget layout configuration patterns and documentation
# Check for layout configuration patterns
echo "Searching for other widget layout configurations..."
rg -A 3 'widgetLayout:\s*defaultLocation:'
# Check for zone/section/area definitions or documentation
echo "Searching for layout zone definitions..."
rg -A 5 'zone:\s*(outer|inner)'
# Check for any layout-related type definitions
echo "Searching for layout type definitions..."
ast-grep --pattern 'type $_ = {
$$$
zone?: $_
$$$
}'
Length of output: 5227
Script:
#!/bin/bash
# Description: Verify sections and areas in widget layout configurations
# Check for section definitions and usages
echo "Searching for section definitions..."
rg -A 3 'section:\s*(left|right|center)'
# Check for area definitions and usages
echo "Searching for area definitions..."
rg -A 3 'area:\s*(top|bottom)'
Length of output: 2838
web/src/beta/features/PluginPlayground/Code/index.tsx (2)
25-30: LGTM! Clean component signature
The component signature follows React best practices with proper TypeScript typing and clean props destructuring.
8-9: Verify handling of undefined sourceCode values
The onChangeSourceCode callback accepts undefined as a valid value. Ensure that the parent component properly handles this case to prevent potential runtime issues.
web/src/beta/features/PluginPlayground/hooks.tsx (2)
21-21: LGTM!
The addition of updateFileSourceCode aligns well with the PR's objective of implementing plugin reflection.
38-38: LGTM!
The Viewer component props and dependency array are correctly updated to handle the widgets prop.
Also applies to: 41-41
web/src/beta/features/PluginPlayground/Plugins/hook.ts (1)
239-239: LGTM!
The new function is properly exposed in the return object, maintaining consistency with the existing pattern.
| const { widgets, executeCode } = useCode({ | ||
| files: selectedPlugin.files | ||
| }); |
There was a problem hiding this comment.
Add null check for selectedPlugin
The code assumes selectedPlugin is always defined. This could lead to runtime errors if selectedPlugin is null/undefined.
Consider adding a null check or providing default values:
const { widgets, executeCode } = useCode({
- files: selectedPlugin.files
+ files: selectedPlugin?.files ?? []
});📝 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.
| const { widgets, executeCode } = useCode({ | |
| files: selectedPlugin.files | |
| }); | |
| const { widgets, executeCode } = useCode({ | |
| files: selectedPlugin?.files ?? [] | |
| }); |
💡 Codebase verification
Missing Error Handling and Unused executeCode Function
The executeCode function is defined but not utilized within the PluginPlayground feature. Additionally, there is no error handling implemented, which may lead to unhandled exceptions.
🔗 Analysis chain
Verify the code execution architecture
Let's verify that the code execution implementation aligns with the intended architecture and handles all edge cases properly.
Also applies to: 97-103
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the code execution implementation
# Check for error handling patterns in related files
echo "Checking error handling patterns..."
rg -A 5 "try.*catch" "./web/src/beta/features/PluginPlayground"
# Check for execution-related event handlers
echo "Checking execution-related handlers..."
ast-grep --pattern 'executeCode($$$)'
# Check for proper cleanup
echo "Checking cleanup patterns..."
rg -A 5 "useEffect.*return.*" "./web/src/beta/features/PluginPlayground"
Length of output: 411
Script:
#!/bin/bash
# Description: Further verify the code execution implementation
# Search for the definition of executeCode
echo "Searching for the definition of executeCode..."
rg "function executeCode" "./web/src/beta/features/PluginPlayground"
rg "const executeCode" "./web/src/beta/features/PluginPlayground"
# Search for all usages of executeCode
echo "Searching for all usages of executeCode..."
rg "executeCode\(" "./web/src/beta/features/PluginPlayground"
# Check for custom error handling beyond try-catch
echo "Checking for custom error handling mechanisms..."
rg -A 5 "errorHandler" "./web/src/beta/features/PluginPlayground"
Length of output: 661
Script:
#!/bin/bash
# Description: Further verify the code execution implementation
# Find all instances where executeCode is passed as a prop or callback
echo "Searching for props or callbacks using executeCode..."
rg "executeCode\s*," ./web/src/beta/features/PluginPlayground
rg "executeCode\s*\)" ./web/src/beta/features/PluginPlayground
# Search for custom error handling implementations
echo "Searching for custom error handling implementations..."
rg "handleError" ./web/src/beta/features/PluginPlayground
rg "onError" ./web/src/beta/features/PluginPlayground
# Verify the dependencies array of useCallback for executeCode
echo "Inspecting dependencies of useCallback for executeCode..."
ast-grep --pattern 'useCallback($_, [$$$]) { executeCode($_) }' ./web/src/beta/features/PluginPlayground/hooks.tsx
Length of output: 916
|
|
||
| const ymlJson = getYmlResult.data; | ||
|
|
||
| if (!ymlJson.extensions) return; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure that ymlJson.extensions is a non-empty array before proceeding.
Before using ymlJson.extensions, it's good practice to verify that it's a non-empty array to prevent runtime errors.
Apply this diff to add the check:
-if (!ymlJson.extensions) return;
+if (!Array.isArray(ymlJson.extensions) || ymlJson.extensions.length === 0) {
+ setNotification({ type: "error", text: "No extensions found in YAML file." });
+ return;
+}📝 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 (!ymlJson.extensions) return; | |
| if (!Array.isArray(ymlJson.extensions) || ymlJson.extensions.length === 0) { | |
| setNotification({ type: "error", text: "No extensions found in YAML file." }); | |
| return; | |
| } |
There was a problem hiding this comment.
We could add this just to double-check
There was a problem hiding this comment.
@mulengawilfred, sounds good! Adding this check will help ensure that ymlJson.extensions is a non-empty array before proceeding, which will prevent potential runtime errors.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
| const outputs = jsFiles.map((file) => { | ||
| console.log(Function(file.sourceCode)); | ||
|
|
||
| try { | ||
| return { | ||
| title: file.title, | ||
| output: Function(file.sourceCode) | ||
| }; | ||
| } catch (error) { | ||
| if (error instanceof Error) { | ||
| return `Error in ${file.title}: ${error.message}`; | ||
| } | ||
| return "Unknown error"; | ||
| } | ||
| }); |
There was a problem hiding this comment.
Remove or utilize the unused variable outputs.
The variable outputs is assigned but never used, which may indicate redundant code or an incomplete implementation.
Apply this diff to remove the unused variable:
-const outputs = jsFiles.map((file) => {
+jsFiles.forEach((file) => {Alternatively, if outputs is intended for future use, ensure it's utilized appropriately in your code.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Check: ci-web / ci
[failure] 60-60:
'outputs' is assigned a value but never used. Allowed unused vars must match /^_/u
There was a problem hiding this comment.
This console.log and outputs don't seem to be used. I am not if outputs is intended to be used.
There was a problem hiding this comment.
@mulengawilfred, you're absolutely right—the console.log statement and the outputs variable aren't being used in the current code. To clean up the code, you can remove them. Here's how you might adjust the code:
jsFiles.forEach((file) => {
try {
Function(file.sourceCode);
} catch (error) {
if (error instanceof Error) {
// Handle the error appropriately
console.error(`Error in ${file.title}: ${error.message}`);
} else {
console.error("Unknown error");
}
}
});This revision executes the code in each file without assigning the result to an unused variable, keeping your codebase clean and efficient.
| const widgets = ymlJson.extensions.reduce<NonNullable<Widgets>>( | ||
| (prv, cur) => { | ||
| const file = files.find((file) => file.title === `${cur.id}.js`); | ||
|
|
||
| if (!file) { | ||
| return prv; | ||
| } | ||
|
|
||
| const { zone, section, area } = cur.widgetLayout?.defaultLocation ?? { | ||
| zone: "outer", | ||
| section: "left", | ||
| area: "top" | ||
| }; | ||
|
|
||
| const zoneAlignSystem = prv?.alignSystem?.[zone] ?? {}; | ||
| const sectionAlignSystem = zoneAlignSystem[section] ?? {}; | ||
| const areaAlignSystem = sectionAlignSystem[area] ?? { widgets: [] }; | ||
|
|
||
| return { | ||
| ...prv, | ||
| alignSystem: { | ||
| ...prv?.alignSystem, | ||
| [zone]: { | ||
| ...zoneAlignSystem, | ||
| [section]: { | ||
| ...sectionAlignSystem, | ||
| [area]: { | ||
| ...areaAlignSystem, | ||
| widgets: [ | ||
| ...(areaAlignSystem.widgets ?? []), | ||
| { | ||
| id: cur.id, | ||
| __REEARTH_SOURCECODE: file.sourceCode | ||
| } | ||
| ] | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }; | ||
| }, | ||
| { | ||
| alignSystem: {}, | ||
| floating: [], | ||
| ownBuiltinWidgets: [] | ||
| } | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid using spread syntax on accumulators within reduce to enhance performance.
Using the spread operator (...) on the accumulator inside a reduce function can lead to O(n²) time complexity, negatively affecting performance.
Refactor the code to mutate the accumulator directly:
-const widgets = ymlJson.extensions.reduce<NonNullable<Widgets>>(
- (prv, cur) => {
- // ...spreading accumulator
- return {
- ...prv,
- // nested spreading
- };
- },
- {
- alignSystem: {},
- floating: [],
- ownBuiltinWidgets: [],
- }
-);
+const widgets: NonNullable<Widgets> = {
+ alignSystem: {},
+ floating: [],
+ ownBuiltinWidgets: [],
+};
+
+ymlJson.extensions.forEach((cur) => {
+ const file = files.find((file) => file.title === `${cur.id}.js`);
+ if (!file) {
+ return;
+ }
+ const { zone, section, area } = cur.widgetLayout?.defaultLocation ?? {
+ zone: "outer",
+ section: "left",
+ area: "top",
+ };
+ const areaWidgets = widgets.alignSystem[zone]?.[section]?.[area]?.widgets ?? [];
+ areaWidgets.push({
+ id: cur.id,
+ __REEARTH_SOURCECODE: file.sourceCode,
+ });
+ widgets.alignSystem[zone] = widgets.alignSystem[zone] || {};
+ widgets.alignSystem[zone][section] = widgets.alignSystem[zone][section] || {};
+ widgets.alignSystem[zone][section][area] = {
+ widgets: areaWidgets,
+ };
+});This approach avoids unnecessary object spreads and enhances performance.
📝 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.
| const widgets = ymlJson.extensions.reduce<NonNullable<Widgets>>( | |
| (prv, cur) => { | |
| const file = files.find((file) => file.title === `${cur.id}.js`); | |
| if (!file) { | |
| return prv; | |
| } | |
| const { zone, section, area } = cur.widgetLayout?.defaultLocation ?? { | |
| zone: "outer", | |
| section: "left", | |
| area: "top" | |
| }; | |
| const zoneAlignSystem = prv?.alignSystem?.[zone] ?? {}; | |
| const sectionAlignSystem = zoneAlignSystem[section] ?? {}; | |
| const areaAlignSystem = sectionAlignSystem[area] ?? { widgets: [] }; | |
| return { | |
| ...prv, | |
| alignSystem: { | |
| ...prv?.alignSystem, | |
| [zone]: { | |
| ...zoneAlignSystem, | |
| [section]: { | |
| ...sectionAlignSystem, | |
| [area]: { | |
| ...areaAlignSystem, | |
| widgets: [ | |
| ...(areaAlignSystem.widgets ?? []), | |
| { | |
| id: cur.id, | |
| __REEARTH_SOURCECODE: file.sourceCode | |
| } | |
| ] | |
| } | |
| } | |
| } | |
| } | |
| }; | |
| }, | |
| { | |
| alignSystem: {}, | |
| floating: [], | |
| ownBuiltinWidgets: [] | |
| } | |
| ); | |
| const widgets: NonNullable<Widgets> = { | |
| alignSystem: {}, | |
| floating: [], | |
| ownBuiltinWidgets: [], | |
| }; | |
| ymlJson.extensions.forEach((cur) => { | |
| const file = files.find((file) => file.title === `${cur.id}.js`); | |
| if (!file) { | |
| return; | |
| } | |
| const { zone, section, area } = cur.widgetLayout?.defaultLocation ?? { | |
| zone: "outer", | |
| section: "left", | |
| area: "top", | |
| }; | |
| const areaWidgets = widgets.alignSystem[zone]?.[section]?.[area]?.widgets ?? []; | |
| areaWidgets.push({ | |
| id: cur.id, | |
| __REEARTH_SOURCECODE: file.sourceCode, | |
| }); | |
| widgets.alignSystem[zone] = widgets.alignSystem[zone] || {}; | |
| widgets.alignSystem[zone][section] = widgets.alignSystem[zone][section] || {}; | |
| widgets.alignSystem[zone][section][area] = { | |
| widgets: areaWidgets, | |
| }; | |
| }); |
🧰 Tools
🪛 Biome
[error] 108-108: 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)
There was a problem hiding this comment.
Worth considering this change as well
There was a problem hiding this comment.
@mulengawilfred, glad you find the suggestion helpful. Let me know if you have any questions or need further assistance.
There was a problem hiding this comment.
Looks good. Added a few issues identified by code-rabbit. Kindly take a look at those potential issues and see which ones need to be resolved.
Also if we can, lets add a to yml in variable names. e.g ReearthYML -> ReearthYAML , getYmlResult -> getYamlResult, ymlJson -> yamlJson.
The minor extra verbosity can bring more clarity to the reader.
The eslint ci-build seems to fail for this but mention me or Shaun to investigate if the fix is not obvious as we have quite a number of issues there.
| const outputs = jsFiles.map((file) => { | ||
| console.log(Function(file.sourceCode)); | ||
|
|
||
| try { | ||
| return { | ||
| title: file.title, | ||
| output: Function(file.sourceCode) | ||
| }; | ||
| } catch (error) { | ||
| if (error instanceof Error) { | ||
| return `Error in ${file.title}: ${error.message}`; | ||
| } | ||
| return "Unknown error"; | ||
| } | ||
| }); |
There was a problem hiding this comment.
This console.log and outputs don't seem to be used. I am not if outputs is intended to be used.
|
|
||
| const ymlJson = getYmlResult.data; | ||
|
|
||
| if (!ymlJson.extensions) return; |
There was a problem hiding this comment.
We could add this just to double-check
| const widgets = ymlJson.extensions.reduce<NonNullable<Widgets>>( | ||
| (prv, cur) => { | ||
| const file = files.find((file) => file.title === `${cur.id}.js`); | ||
|
|
||
| if (!file) { | ||
| return prv; | ||
| } | ||
|
|
||
| const { zone, section, area } = cur.widgetLayout?.defaultLocation ?? { | ||
| zone: "outer", | ||
| section: "left", | ||
| area: "top" | ||
| }; | ||
|
|
||
| const zoneAlignSystem = prv?.alignSystem?.[zone] ?? {}; | ||
| const sectionAlignSystem = zoneAlignSystem[section] ?? {}; | ||
| const areaAlignSystem = sectionAlignSystem[area] ?? { widgets: [] }; | ||
|
|
||
| return { | ||
| ...prv, | ||
| alignSystem: { | ||
| ...prv?.alignSystem, | ||
| [zone]: { | ||
| ...zoneAlignSystem, | ||
| [section]: { | ||
| ...sectionAlignSystem, | ||
| [area]: { | ||
| ...areaAlignSystem, | ||
| widgets: [ | ||
| ...(areaAlignSystem.widgets ?? []), | ||
| { | ||
| id: cur.id, | ||
| __REEARTH_SOURCECODE: file.sourceCode | ||
| } | ||
| ] | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }; | ||
| }, | ||
| { | ||
| alignSystem: {}, | ||
| floating: [], | ||
| ownBuiltinWidgets: [] | ||
| } | ||
| ); |
There was a problem hiding this comment.
Worth considering this change as well
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
web/src/beta/features/PluginPlayground/Code/hook.ts(1 hunks)
🧰 Additional context used
🪛 Biome
web/src/beta/features/PluginPlayground/Code/hook.ts
[error] 96-96: 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)
| const executeCode = useCallback(() => { | ||
| const ymlFile = files.find((file) => file.title.endsWith(".yml")); | ||
|
|
||
| if (!ymlFile) return; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error notification when no YAML file is found
Currently, if no YAML file is found in the provided files, the function returns silently without notifying the user. It would be helpful to inform the user that no YAML file was found to provide better feedback.
Apply this diff to add an error notification:
if (!ymlFile) {
+ setNotification({ type: "error", text: "No YAML file found." });
return;
}Committable suggestion skipped: line range outside the PR's diff.
Overview
What I've done
I implemented the process to execute reearth.yml and other JS files and reflect the plugins in the visualizer.
2024-11-07.22.12.10.mov
What I haven't done
How I tested
Which point I want you to review particularly
Memo
Summary by CodeRabbit
Release Notes
New Features
Codecomponent for improved interaction with source code changes and execution capabilities.Improvements
Viewercomponent.Bug Fixes