feat: State Inspector #38368
feat: State Inspector #38368
Conversation
WalkthroughThis pull request introduces a comprehensive State Inspector feature for the debugger, enabling users to view and interact with various application entities. The implementation spans multiple files, adding new components, hooks, and Redux state management to support a detailed view of application state across widgets, queries, JS objects, and global entities. Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
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
|
|
/build-deploy-preview |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/12500191168. |
|
This PR has increased the number of cyclic dependencies by 3, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/client/src/components/editorComponents/Debugger/StateInspector/StateInspector.tsx (1)
24-73: Neat and well-organized state inspection component.The layout and usage of hooks, combined with
react-json-view, form a coherent UI to inspect application data. Consider unit tests to verify item selection and data filtering logic.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
app/client/src/IDE/Components/BottomView.tsx(2 hunks)app/client/src/ce/constants/messages.ts(1 hunks)app/client/src/ce/entities/IDE/constants.ts(1 hunks)app/client/src/ce/pages/Editor/IDE/EditorPane/JS/ListItem.tsx(0 hunks)app/client/src/ce/selectors/entitiesSelector.ts(3 hunks)app/client/src/components/editorComponents/Debugger/DebuggerTabs.tsx(2 hunks)app/client/src/components/editorComponents/Debugger/StateInspector/StateInspector.tsx(1 hunks)app/client/src/components/editorComponents/Debugger/StateInspector/hooks.ts(1 hunks)app/client/src/components/editorComponents/Debugger/StateInspector/index.ts(1 hunks)app/client/src/components/editorComponents/Debugger/constants.ts(1 hunks)app/client/src/pages/Editor/Explorer/ExplorerIcons.tsx(3 hunks)app/client/src/pages/Editor/Explorer/Files/index.tsx(0 hunks)app/client/src/pages/Editor/Explorer/JSActions/JSActionEntity.tsx(0 hunks)
💤 Files with no reviewable changes (3)
- app/client/src/ce/pages/Editor/IDE/EditorPane/JS/ListItem.tsx
- app/client/src/pages/Editor/Explorer/JSActions/JSActionEntity.tsx
- app/client/src/pages/Editor/Explorer/Files/index.tsx
✅ Files skipped from review due to trivial changes (1)
- app/client/src/components/editorComponents/Debugger/StateInspector/index.ts
🔇 Additional comments (17)
app/client/src/components/editorComponents/Debugger/constants.ts (1)
7-7: Looks Good – Enum entry for the new State tab.
No issues found with adding this enum entry for STATE_TAB.
app/client/src/components/editorComponents/Debugger/StateInspector/StateInspector.tsx (1)
10-22: Helpful customization for JSON visualization.
reactJsonProps provides a clean display configuration for JSON data, enhancing readability and user experience.
app/client/src/components/editorComponents/Debugger/StateInspector/hooks.ts (1)
12-95: Clean hook design for state inspection.
The arrangement into categories (Queries, JS objects, UI elements, and Globals) is logical and straightforward. Good job keeping the internal state isolated.
app/client/src/components/editorComponents/Debugger/DebuggerTabs.tsx (3)
20-20: Localized naming for the State tab.
Importing DEBUGGER_STATE ensures consistent UI text across the application.
26-26: Statically importing the StateInspector for clarity.
This import approach is clear and keeps the file organized.
62-66: Seamless tab addition for the StateInspector.
The new State tab is integrated smoothly into the existing tab structure, expanding debugger functionality without disruption.
app/client/src/IDE/Components/BottomView.tsx (2)
Line range hint 31-38: Neat CSS formatting for the tabs list.
This provides clarity and consistent spacing.
43-43: Consider verifying 'user-select: none'.
It might hamper text selection for users who want to copy tab labels.
app/client/src/pages/Editor/Explorer/ExplorerIcons.tsx (3)
15-17: New widget-related imports look spot on.
365-369: Dynamic widget icon creation is concise.
This approach cleanly defaults to a generic icon if a custom icon is unavailable.
371-373: Minimal & straightforward GlobeIcon.
No issues at a glance.
app/client/src/ce/selectors/entitiesSelector.ts (4)
26-29: New imports from WidgetConstants.
No issues here, matches usage references.
65-69: Entity item imports for IDE constants.
Cleanly typed; helps maintain clarity in entity usage.
73-73: Importing WidgetIconByType.
Ensures the utility is accessible for UI segmentation.
990-1001: getUISegmentItems offers a neat aggregator.
Filters out main container and attaches an icon to each widget. Looks good for populating UI segments.
app/client/src/ce/constants/messages.ts (1)
567-567: DEBUGGER_STATE constant addition.
Straightforward. Helps label the debug tab for state info.
app/client/src/ce/entities/IDE/constants.ts (1)
132-133: Make sure usage references remain intact after omitting type.
By omitting the type property from EntityItem, any code expecting type might break. Verify that no existing references rely on this field or adjust accordingly.
✅ Verification successful
Let me check how the EntityItem interface is defined and how GenericEntityItem is being used.
Omitting type property is safe in this context
From the verification results:
EntityIteminterface has atype: PluginTypefield that's being omittedGenericEntityItemis only used in the StateInspector component where:- It's used as a type for state management
- No references to
.typeproperty were found - The initialization shows only
keyandtitleproperties being used
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify that no existing code references `GenericEntityItem` expecting a `type` property
rg -A 5 -B 5 'GenericEntityItem'
Length of output: 4747
Script:
#!/bin/bash
# Check the definition of EntityItem and its usages
ast-grep --pattern 'interface EntityItem {
$$$
}'
# Also check if there are any direct property accesses on GenericEntityItem
rg '\.type' -C 5 app/client/src/components/editorComponents/Debugger/StateInspector/
Length of output: 676
|
Deploy-Preview-URL: https://ce-38368.dp.appsmith.com |
|
This PR has increased the number of cyclic dependencies by 3, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. |
1 similar comment
|
This PR has increased the number of cyclic dependencies by 3, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
app/client/src/components/editorComponents/Debugger/StateInspector/hooks.ts (3)
19-22: Consider a configurable default selection
Currently, the default selection is “appsmith”. If there's a requirement to dynamically customize the default selected item, consider exposing it through a hook parameter or context.
25-61: DRY opportunity across item mappings
Query, JS, and widget items share a similar mapping pattern. Extracting this logic into a helper function could reduce duplication and improve maintainability.
63-101: Unshift-based ordering could be unclear
Unshifting array elements in reverse priority can be confusing. Directly assembling the array in the desired order might be clearer for future maintainers.app/client/src/components/editorComponents/Debugger/StateInspector/StateInspector.tsx (2)
11-23: ReactJson customization
These props provide a clear and concise JSON visualization. Consider enabling clipboard if users need to copy data for deeper investigation.
25-84: Search input usage
TheSearchInputplaceholder is present, but there's no evident filtering logic yet. If search is planned, connect it to a data filtering mechanism so users can find entities quickly.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/client/src/ce/PluginActionEditor/components/PluginActionResponse/hooks/usePluginActionResponseTabs.tsx(3 hunks)app/client/src/ce/constants/messages.ts(1 hunks)app/client/src/components/editorComponents/Debugger/StateInspector/StateInspector.tsx(1 hunks)app/client/src/components/editorComponents/Debugger/StateInspector/hooks.ts(1 hunks)app/client/src/components/editorComponents/Debugger/StateInspector/styles.ts(1 hunks)app/client/src/components/editorComponents/JSResponseView.tsx(3 hunks)app/client/src/pages/Editor/Explorer/ExplorerIcons.tsx(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- app/client/src/components/editorComponents/Debugger/StateInspector/styles.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- app/client/src/pages/Editor/Explorer/ExplorerIcons.tsx
- app/client/src/ce/constants/messages.ts
🔇 Additional comments (6)
app/client/src/components/editorComponents/Debugger/StateInspector/hooks.ts (2)
12-18: Cleanly defined hook signature
This hook neatly returns the currently selected item and the grouped items array, tailored for the state inspector. Its return type is explicit and easy to consume.
103-104: Return structure is well-designed
Returning a tuple with [selectedItem, groups] is clear and fits common React patterns, making it simple to destructure in consumers.
app/client/src/ce/PluginActionEditor/components/PluginActionResponse/hooks/usePluginActionResponseTabs.tsx (3)
14-14: Appropriate use of new message constant
Importing DEBUGGER_STATE from ee/constants/messages is consistent with the existing pattern for debugger tab labels.
36-36: Proper integration of StateInspector
Adding StateInspector helps users explore application state alongside other debugger features.
163-167: Conditional tab initialization
The new State tab is only available in FullScreen mode, which might be a design requirement. Confirm whether there's a need to support smaller view modes.
app/client/src/components/editorComponents/JSResponseView.tsx (1)
208-220: Seamless addition of the new State tab
Appending the STATE_TAB in fullscreen mode aligns with existing logs and errors tabs for debugging.
|
/build-deploy-preview |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/12512656937. |
|
Deploy-Preview-URL: https://ce-38368.dp.appsmith.com |
|
This PR has increased the number of cyclic dependencies by 3, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/client/src/ce/PluginActionEditor/components/PluginActionResponse/hooks/usePluginActionResponseTabs.tsx (1)
Line range hint
1-61: LGTM! Consider improving type safety for ragDocumentsCount.The hook setup and imports look good. Consider adding explicit type for
ragDocumentsCountstate.- const [ragDocumentsCount, setRagDocumentsCount] = useState<number>(); + const [ragDocumentsCount, setRagDocumentsCount] = useState<number | undefined>(undefined);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/client/src/ce/PluginActionEditor/components/PluginActionResponse/hooks/usePluginActionResponseTabs.tsx(5 hunks)app/client/src/ce/selectors/entitiesSelector.ts(3 hunks)app/client/src/components/editorComponents/Debugger/DebuggerTabs.tsx(3 hunks)app/client/src/components/editorComponents/Debugger/constants.ts(1 hunks)app/client/src/components/editorComponents/JSResponseView.tsx(6 hunks)app/client/src/pages/Editor/Explorer/ExplorerIcons.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/client/src/components/editorComponents/Debugger/constants.ts
- app/client/src/components/editorComponents/Debugger/DebuggerTabs.tsx
👮 Files not reviewed due to content moderation or server errors (3)
- app/client/src/pages/Editor/Explorer/ExplorerIcons.tsx
- app/client/src/components/editorComponents/JSResponseView.tsx
- app/client/src/ce/selectors/entitiesSelector.ts
🧰 Additional context used
🪛 Biome (1.9.4)
app/client/src/components/editorComponents/JSResponseView.tsx
[error] 93-93: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 94-94: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 95-95: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (2)
app/client/src/ce/PluginActionEditor/components/PluginActionResponse/hooks/usePluginActionResponseTabs.tsx (2)
Line range hint 204-229: LGTM! Verify IDE type detection across routes.
The state inspector integration looks good. Let's verify the IDE type detection across different routes:
✅ Verification successful
IDE type detection implementation is consistent and correct
The IDE type detection is properly implemented across the codebase:
getIDETypeByUrlfunction is consistently used for IDE type detectionIDE_TYPE.Apppaths are correctly defined inIDEBasePathsconstant- The state inspector tab is consistently shown only for App IDE type across multiple components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for IDE type detection usage patterns
ast-grep --pattern 'getIDETypeByUrl($path)'
# Search for IDE_TYPE.App usage to ensure consistent behavior
rg "IDE_TYPE.App" -A 2
Length of output: 45
Script:
#!/bin/bash
# Search for IDE type detection function definition and usage
rg "getIDETypeByUrl" -A 3
# Search for IDE type enum or constant definitions
rg "IDE_TYPE" -A 3
# Search for route patterns that might affect IDE type
rg "IDE_TYPE.*App" -A 3
Length of output: 18650
156-202: Improve type safety for datasource handling.
The AI plugin type handling looks good, but there are some type safety concerns:
- datasourceId={(datasource as DatasourceType)?.id}
+ datasourceId={datasource && 'id' in datasource ? datasource.id : undefined}Let's verify the Anvil feature flag usage:
✅ Verification successful
Let me verify the feature flag implementation further by checking the selector implementation and its usage in the codebase.
Let me check how the layout system type is determined.
Based on the verification results, I can now provide a final response about the feature flag implementation and type safety concerns.
Feature flag implementation is correct, type casting can be improved
The isAnvilEnabled feature flag is consistently used across the codebase through the getIsAnvilEnabledInCurrentApplication selector, which properly checks the layout system type. The type casting concern can be addressed with a safer approach:
- The current type cast
(datasource as DatasourceType)?.idshould be improved - Use type guard pattern instead:
datasourceId={datasource && 'id' in datasource ? datasource.id : undefined}🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other usages of isAnvilEnabled to ensure consistent behavior
rg "getIsAnvilEnabledInCurrentApplication" -A 2
Length of output: 5593
Script:
#!/bin/bash
# Check the selector implementation
ast-grep --pattern 'export const getIsAnvilEnabledInCurrentApplication = createSelector($$$)'
# Check for any related constants or types
rg "LayoutSystemTypes" -A 2
Length of output: 681
Script:
#!/bin/bash
# Check for layout system type definition and usage
rg "enum LayoutSystemTypes" -A 5
# Check for getLayoutSystemType implementation
ast-grep --pattern 'export const getLayoutSystemType = $$$'
Length of output: 77982
|
This PR has increased the number of cyclic dependencies by 3, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (8)
app/client/src/components/editorComponents/Debugger/StateInspector/StateInspector.tsx (3)
12-24: Consider memoizing reactJsonProps.The props object is recreated on every render. Consider moving it outside the component or using useMemo.
-export const reactJsonProps = { +export const reactJsonProps = React.useMemo(() => ({ name: null, enableClipboard: false, displayDataTypes: false, displayArrayKey: true, quotesOnKeys: false, style: { fontSize: "12px", }, collapsed: 1, indentWidth: 2, collapseStringsAfterLength: 30, -}; +}), []);
33-43: Improve type safety and initialization.The
filteredDatavariable could benefit from a more specific type and better initialization.-let filteredData: unknown = ""; +let filteredData: Record<string, unknown> = {};
47-107: Consider splitting the component for better maintainability.The component handles multiple responsibilities (search, list, and data display). Consider splitting it into smaller, focused components.
- Extract the sidebar into a separate
StateInspectorSidebarcomponent- Extract the main content into a
StateInspectorContentcomponentapp/client/src/components/editorComponents/Debugger/StateInspector/hooks.ts (2)
34-70: Extract common item properties to reduce duplication.The mapping logic for queries, JS objects, and widgets contains duplicated properties.
const createListItem = (item: GenericEntityItem, isSelected: boolean): ListItemProps => ({ id: item.key, title: item.title, startIcon: item.icon, isSelected, onClick: () => setSelectedItem(item), description: "", descriptionType: "inline", size: "md", });
96-115: Simplify groups array manipulation.Consider using array methods instead of multiple unshift operations.
const groupsData = [ queryItems.length && { group: "Queries", items: queryItems }, jsItems.length && { group: "JS objects", items: jsItems }, widgetItems.length && { group: "UI elements", items: widgetItems }, { group: "Globals", items: [globalListItem] } ].filter(Boolean);app/client/src/components/editorComponents/JSResponseView.tsx (3)
65-73: Props interface looks good but could be stricterThe Props interface is well-defined, but consider making optional props explicit with the
?operator for better type safety.interface Props { currentFunction: JSAction | null; - theme?: EditorTheme; + theme: EditorTheme | undefined; errors: Array<EvaluationError>; disabled: boolean; isLoading: boolean; onButtonClick: (e: React.MouseEvent<HTMLElement, MouseEvent>) => void; jsCollectionData: JSCollectionData | undefined; - debuggerLogsDefaultName?: string; + debuggerLogsDefaultName: string | undefined; }
91-97: Optimize object destructuring with optional chainingThe destructuring can be simplified using optional chaining for better readability and maintainability.
const { isDirty, isExecuting, responses } = useMemo(() => { return { - responses: (jsCollectionData && jsCollectionData.data) || {}, - isDirty: (jsCollectionData && jsCollectionData.isDirty) || {}, - isExecuting: (jsCollectionData && jsCollectionData.isExecuting) || {}, + responses: jsCollectionData?.data ?? {}, + isDirty: jsCollectionData?.isDirty ?? {}, + isExecuting: jsCollectionData?.isExecuting ?? {}, }; }, [jsCollectionData]);🧰 Tools
🪛 Biome (1.9.4)
[error] 93-93: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 94-94: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 95-95: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
137-210: Consider breaking down JSResponseTab into smaller componentsThe JSResponseTab is quite large with multiple conditional renders. Consider extracting the response status renders into separate components for better maintainability.
For example:
const ExecutingResponse = ({ theme }: { theme?: EditorTheme }) => ( <LoadingOverlayScreen theme={theme}> {createMessage(EXECUTING_FUNCTION)} </LoadingOverlayScreen> ); const NoReturnValueResponse = ({ functionName }: { functionName?: string }) => ( <NoReturnValueWrapper> <Text kind="body-m"> {createMessage(NO_JS_FUNCTION_RETURN_VALUE, functionName)} </Text> </NoReturnValueWrapper> );
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/client/src/actions/debuggerActions.ts(2 hunks)app/client/src/ce/constants/ReduxActionConstants.tsx(1 hunks)app/client/src/components/editorComponents/Debugger/StateInspector/StateInspector.tsx(1 hunks)app/client/src/components/editorComponents/Debugger/StateInspector/hooks.ts(1 hunks)app/client/src/components/editorComponents/Debugger/StateInspector/styles.ts(1 hunks)app/client/src/components/editorComponents/JSResponseView.tsx(5 hunks)app/client/src/reducers/uiReducers/debuggerReducer.ts(4 hunks)app/client/src/selectors/debuggerSelectors.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
app/client/src/components/editorComponents/JSResponseView.tsx
[error] 93-93: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 94-94: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 95-95: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (12)
app/client/src/actions/debuggerActions.ts (2)
10-10: Good enhancement for typed payload imports.
Bringing in GenericEntityItem ensures type safety and clarity, which is beneficial for debugging actions.
151-159: New action creator looks good.
Function setDebuggerStateInspectorSelectedItem properly dispatches an action with a typed payload, helping maintain a clean debugger flow.
app/client/src/selectors/debuggerSelectors.tsx (1)
187-189: Selector introduced for efficient state retrieval.
getDebuggerStateInspectorSelectedItem neatly exposes the selected item in the Debugger’s state inspector for external use.
app/client/src/reducers/uiReducers/debuggerReducer.ts (4)
9-9: Reinforces correct data modeling.
Importing GenericEntityItem is a sensible foundation for typed reducer updates.
26-26: Initial state extension is straightforward.
Adding stateInspector to initialState keeps the selected item management separate and well-scoped.
190-200: Reducer case handling is well-structured.
Updates to stateInspector appear concise and maintain immutability.
214-216: Interface update is consistent.
Including selectedItem in DebuggerReduxState ensures a stronger contract for consumers.
app/client/src/ce/constants/ReduxActionConstants.tsx (1)
700-701: Action type addition aligns with new functionality.
SET_DEBUGGER_STATE_INSPECTOR_SELECTED_ITEM is clearly named and integrates well with existing IDE Debugger action types.
app/client/src/components/editorComponents/Debugger/StateInspector/styles.ts (2)
4-9: LGTM! Well-structured reusable CSS.
The imgSizer CSS fragment is properly defined for reuse across components.
Line range hint 11-24: LGTM! Clean styled-components implementation.
Both styled components properly extend Flex and reuse the imgSizer CSS.
app/client/src/components/editorComponents/JSResponseView.tsx (2)
217-249: LGTM! Well-structured tab configuration
The tab configuration is properly memoized and the conditional logic for showing the state inspector tab is clear and maintainable.
257-269: LGTM! Properly memoized callbacks
The callback handlers are correctly implemented with proper dependency arrays and dispatch inclusion.
app/client/src/components/editorComponents/Debugger/StateInspector/hooks.ts
Outdated
Show resolved
Hide resolved
| // @ts-ignore | ||
| onRunClick={onButtonClick} |
There was a problem hiding this comment.
💡 Codebase verification
🛠️ Refactor suggestion
Fix type mismatch by using correct event type for onRunClick
The onRunClick prop in NoResponse component expects a function with no parameters () => void, but you're passing onButtonClick which has type (e: React.MouseEvent<HTMLElement, MouseEvent>) => void. Update the prop to:
onRunClick={() => onButtonClick}🔗 Analysis chain
Remove @ts-ignore by fixing the type
Instead of bypassing TypeScript with @ts-ignore, consider fixing the type mismatch. This will improve type safety and maintainability.
Please verify the expected type of onRunClick in the NoResponse component and update the type definition accordingly.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find the type definition of NoResponse component
ast-grep --pattern 'interface $name {
$$$
onRunClick: $_
$$$
}'
Length of output: 71
Script:
#!/bin/bash
# Let's try to find the NoResponse component definition and its props type
ast-grep --pattern 'const NoResponse: $_ = $_'
# Also search for any type definitions containing NoResponse
ast-grep --pattern 'type NoResponse'
# Let's also check the file content to understand the context better
rg -A 5 -B 5 "NoResponse" "app/client/src/components/editorComponents/JSResponseView.tsx"
Length of output: 48
Script:
#!/bin/bash
# Let's try to find the file content directly
cat "app/client/src/components/editorComponents/JSResponseView.tsx"
# Also search for NoResponse in other files in case it's imported
rg "NoResponse" --type ts --type tsx
Length of output: 9617
Script:
#!/bin/bash
# Let's find the NoResponse component definition in the PluginActionResponse directory
fd -e tsx -e ts "NoResponse.tsx" "app/client/src/PluginActionEditor"
# Also try to find the file content
rg -l "NoResponse" app/client/src/PluginActionEditor --type typescript
Length of output: 275
Script:
#!/bin/bash
# Let's check the NoResponse component definition
cat "app/client/src/PluginActionEditor/components/PluginActionResponse/components/NoResponse/NoResponse.tsx"
Length of output: 899
|
This PR has increased the number of cyclic dependencies by 3, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/client/packages/design-system/ads/src/List/List.tsx (2)
118-119: Ensure keyboard accessibilityGreat job moving these handlers to
StyledListItem. Consider specifying a suitable ARIA role (e.g.,role="button") so screen readers can accurately recognize the entire item as clickable and keyboard-interactive.
123-123: Reassess the purpose ofContentTextWrapperNow that event handling is elevated to the parent, confirm that
ContentTextWrapperis strictly for styling and layout, and doesn’t contain redundant or contradictory interaction logic.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
app/client/packages/design-system/ads/src/__assets__/icons/ads/globe-simple.svgis excluded by!**/*.svg
📒 Files selected for processing (2)
app/client/packages/design-system/ads/src/Icon/Icon.provider.tsx(1 hunks)app/client/packages/design-system/ads/src/List/List.tsx(1 hunks)
|
This PR has increased the number of cyclic dependencies by 3, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/client/src/ce/selectors/entitiesSelector.ts (1)
990-1000: LGTM! Well-implemented selector with proper memoization.The selector correctly:
- Uses createSelector for memoization
- Filters out the main container widget
- Maps widgets to the required GenericEntityItem format
However, consider adding a type annotation for better maintainability:
-export const getUISegmentItems = createSelector(getCanvasWidgets, (widgets) => { +export const getUISegmentItems = createSelector(getCanvasWidgets, (widgets): GenericEntityItem[] => {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/client/packages/design-system/ads/src/Icon/Icon.provider.tsx(1 hunks)app/client/src/ce/PluginActionEditor/components/PluginActionResponse/hooks/usePluginActionResponseTabs.tsx(4 hunks)app/client/src/ce/selectors/entitiesSelector.ts(3 hunks)app/client/src/selectors/debuggerSelectors.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- app/client/src/selectors/debuggerSelectors.tsx
- app/client/src/ce/PluginActionEditor/components/PluginActionResponse/hooks/usePluginActionResponseTabs.tsx
- app/client/packages/design-system/ads/src/Icon/Icon.provider.tsx
🔇 Additional comments (1)
app/client/src/ce/selectors/entitiesSelector.ts (1)
26-29: LGTM! Required imports for UI segment functionality.
The new imports are properly organized and necessary for the new getUISegmentItems selector implementation.
Also applies to: 65-69, 73-73
github-actions
bot
added
IDE Pod
|
This PR has increased the number of cyclic dependencies by 3, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/client/src/components/editorComponents/Debugger/StateInspector/StateInspector.test.tsx (1)
42-61: Consider adding a test case for null or missing onClick.
While testing a validonClickmethod is great, consider also verifying behavior when an item’sonClickis undefined or null.app/client/src/components/editorComponents/Debugger/StateInspector/StateInspector.tsx (1)
78-98: Suggest fallback rendering for edge cases.
While most items will have validiconortitle, consider conditionally rendering them to avoid potential null references in future expansions.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/client/src/components/editorComponents/Debugger/StateInspector/StateInspector.test.tsx(1 hunks)app/client/src/components/editorComponents/Debugger/StateInspector/StateInspector.tsx(1 hunks)
🔇 Additional comments (14)
app/client/src/components/editorComponents/Debugger/StateInspector/StateInspector.test.tsx (10)
1-7: All imports are set up correctly.
Your import choices for the testing library and mocking utilities look good.
8-10: Mocks for hooks and utilities are clear and maintainable.
Mocking the hooks and utility functions allows for isolated unit testing. Great work!
15-23: Nice approach to mocking the filtering logic.
This ensures your test isn’t reliant on the real implementation and explicitly verifies the contract.
25-41: Thorough test for filtering behavior.
You've covered both the search input's initial render and its effect on displayed items. This is excellent coverage.
62-84: Clear coverage of selected item details.
This test ensures the selected item’s properties are displayed correctly. The assertion is straightforward and effective.
85-90: Good negative coverage for unselected items.
Confirming that the UI hides details when nothing is selected. Nicely done.
91-107: Sufficient test for empty search term.
This verifies that all groups appear when no filter is applied. Excellent demonstration of default behavior.
108-115: Proper test for non-matching searches.
Verifying that no items display when the search doesn't match any group is critical. Well done.
117-122: Solid coverage for empty items list.
Ensures that the component gracefully handles an empty array of items.
124-143: Good handling of missing code in the selected item.
Testing that the UI degrades gracefully when code is null or undefined is a great addition to your coverage.app/client/src/components/editorComponents/Debugger/StateInspector/StateInspector.tsx (4)
1-26: Imports and props configuration look good.
The chosen settings forReactJsonensure a clean display of nested objects.
28-36: Smart separation of concerns for search and item data.
Exposing bothitemsandselectedItemCodethrough your hook is flexible and testable.
60-75: Effective display of item groups.
Mapping through the filtered groups and using aListfor items is straightforward and maintainable.
95-95: Confirm type safety for the code prop.
If there is any possibility of an unexpected data shape (e.g., arrays, strings), add a fallback to avoid potential runtime errors.
|
This PR has increased the number of cyclic dependencies by 3, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. |
app/client/src/components/editorComponents/Debugger/StateInspector/utils.ts
Outdated
Show resolved
Hide resolved
...ponents/editorComponents/Debugger/StateInspector/hooks/useGetGlobalItemsForStateInspector.ts
Show resolved
Hide resolved
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/12596085888. |
|
Deploy-Preview-URL: https://ce-38368.dp.appsmith.com |
|
This PR has increased the number of cyclic dependencies by 3, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
app/client/src/components/editorComponents/Debugger/StateInspector/styles.ts (1)
11-15: Use consistent naming across styled components.Naming the container “Group” is descriptive, but ensure it aligns with naming patterns used elsewhere in the codebase (e.g., “Section” or “Block”).
app/client/src/components/editorComponents/Debugger/StateInspector/StateInspector.tsx (4)
1-2: Combine related imports.Consider grouping imports from the same package to keep the import section more concise.
14-26: Validate “react-json-view” configurations.These props appear correct, though if users often expand JSON objects, reconsider the default collapsed level. Evaluate user stories for optimal defaults.
28-30: Potentially rename local state.Consider making
selectedItem,items, andselectedItemCodeseparate well-labeled state variables or custom types for clarity, as the current naming might cause confusion in larger contexts.
37-99: Enhance user feedback for no selection scenario.Currently, we hide content when
selectedItemis not defined. Consider providing a basic placeholder or informational message describing how to select an item.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/client/src/components/editorComponents/Debugger/StateInspector/StateInspector.tsx(1 hunks)app/client/src/components/editorComponents/Debugger/StateInspector/hooks/useGetGlobalItemsForStateInspector.ts(1 hunks)app/client/src/components/editorComponents/Debugger/StateInspector/styles.ts(1 hunks)app/client/src/components/editorComponents/Debugger/StateInspector/utils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/client/src/components/editorComponents/Debugger/StateInspector/hooks/useGetGlobalItemsForStateInspector.ts
🔇 Additional comments (6)
app/client/src/components/editorComponents/Debugger/StateInspector/utils.ts (2)
1-4: Consider reducing import statements if references are not needed.Ensure that all imported types (
ListItemWithoutOnClick,ListItemProps,GenericEntityItem) are strictly necessary. If not, streamline these imports to enhance maintainability.
5-21: Improve clarity of entity key assignment.The approach of assigning
key: item.idis valid, but double-check any implicit assumptions about the uniqueness ofitem.idto prevent key collisions in downstream code.app/client/src/components/editorComponents/Debugger/StateInspector/styles.ts (3)
4-9: Avoid accidental conflicts with global img rules.The
img { height: 16px; width: 16px; }block inimgSizeris fine, but confirm that it won't override image styling in unrelated components.
17-19: Maintain consistent margin/padding units.The usage of CSS variables is great. Just ensure that this pattern is consistently followed in the rest of the components to maintain uniform spacing throughout the UI.
21-23: Double-check styling for override conflicts.
SelectedItemincludesimgSizer; verify that it does not unintentionally override other component styles in the parent container.app/client/src/components/editorComponents/Debugger/StateInspector/StateInspector.tsx (1)
32-36: Check type arguments on filter function.The usage of generics looks clear. Just confirm that the generic constraints align with the expected shape of
itemsto avoid runtime issues.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/client/src/actions/jsPaneActions.ts (1)
15-15: Optional doc comment recommendation.
You might consider adding a short JSDoc comment to explain howJSUpdateis intended to be used, to help maintain clarity and consistency.app/client/src/sagas/EvaluationsSaga.ts (1)
Line range hint
548-606: Implementation of eval queue buffering.
This approach effectively consolidates action payloads and post-eval actions. However, consider adding incremental logs or checks to ensure no race conditions occur when merging JS object updates.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/client/src/actions/jsPaneActions.ts(3 hunks)app/client/src/actions/pluginActionActions.ts(1 hunks)app/client/src/sagas/ActionExecution/PluginActionSaga.ts(1 hunks)app/client/src/sagas/EvaluationsSaga.ts(2 hunks)app/client/src/sagas/JSPaneSagas.ts(1 hunks)
🔇 Additional comments (6)
app/client/src/actions/jsPaneActions.ts (2)
1-4: Imports updated successfully.
139-144: Validate usage in codebase.
Please ensure that consumers ofexecuteJSUpdateshandle the unknown return type properly, especially if this action triggers asynchronous results or external side effects.app/client/src/actions/pluginActionActions.ts (1)
Line range hint
394-406: New function looks good.
This function’s definition forupdateActionDataappears straightforward. Confirm that the payload’s structure matches the saga expectations to avoid runtime shape mismatches.app/client/src/sagas/JSPaneSagas.ts (1)
931-931: Added saga listener for JSUpates looks consistent.
This line properly hooks intoEXECUTE_JS_UPDATES. Ensure that all side effects triggered bymakeUpdateJSCollectionare covered by unit tests.app/client/src/sagas/EvaluationsSaga.ts (1)
101-101: Switched import reference is correct.
ImportingexecuteJSUpdatesfromjsPaneActionsinstead ofpluginActionActionsaligns with the reorganization and is consistent.app/client/src/sagas/ActionExecution/PluginActionSaga.ts (1)
21-21: Validate the removal ofmakeUpdateJSCollectionand the addition ofhandleExecuteJSFunctionSaga.
It appears thatmakeUpdateJSCollectionwas removed, and this new import may serve similar or related functionality. Verify that no references to the old function remain, and ensure that the newly importedhandleExecuteJSFunctionSagais indeed what's intended for the updated execution flow.
Description
Adds a state inspector inside the Debugger of App IDE
Fixes #38016
Automation
/ok-to-test tags="@tag.Sanity"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12596980773
Commit: 8b2cc89
Cypress dashboard.
Tags:
@tag.SanitySpec:
Fri, 03 Jan 2025 12:09:45 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes