feat: Implement infra code for infinite scroll implementation. #39225
Conversation
- Adding `infiniteScrollEnabled` prop to TableWidgetProps
…o rahulbarwal/issue39079/Feature-flag-to-control-visibility-of-this-feature
Implements infinite scroll functionality for table widget using react-window-infinite-loader. Introduces new components and hooks to manage virtualized table rendering with dynamic loading of rows.
WalkthroughThis pull request enhances the table components to support infinite scrolling. It adds new dependencies for infinite loader support and updates several components—including StaticTable, Table, VirtualTable, and TableBody—to accept new properties ( Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant TB as TableBody
participant ISB as InfiniteScrollBody
participant IV as useInfiniteVirtualization
participant API as DataLoader
U->>ISB: Scroll event near bottom
ISB->>IV: Trigger loadMoreItems callback
IV->>TB: Invoke loadMoreFromEvaluations
TB->>API: Fetch additional data
API-->>TB: Return updated rows
TB-->>ISB: Update rendered list
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (7)
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
|
rahulbarwal
changed the base branch from
rahulbarwal/issue39079/Feature-flag-to-control-visibility-of-this-feature
to
release
github-actions
bot
added
Enhancement
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
app/client/src/widgets/TableWidgetV2/component/TableBody/InifiniteScrollBody/useInfiniteVirtualization.tsx (1)
24-30: Consider utilizing startIndex/stopIndex parameters in loadMoreItems.The loadMoreItems callback ignores the startIndex and stopIndex parameters which could be useful for optimizing the loading of specific ranges of data.
- const loadMoreItems = useCallback(async () => { + const loadMoreItems = useCallback(async (startIndex: number, stopIndex: number) => { if (!isLoading) { - loadMore(); + loadMore(startIndex, stopIndex); } return Promise.resolve(); }, [isLoading, loadMore]);app/client/src/widgets/TableWidgetV2/component/TableBody/InifiniteScrollBody/index.tsx (1)
38-38: Remove hardcoded buffer size.The itemCount + 5 is using a magic number. Consider making this configurable or calculating based on viewport size.
- itemCount={itemCount + 5} + itemCount={itemCount + Math.ceil(height / tableSizes.ROW_HEIGHT)}app/client/src/widgets/TableWidgetV2/component/TableBody/VirtualList.tsx (1)
61-61: Consider caching itemCount calculation.The itemCount calculation could be memoized to prevent unnecessary recalculations.
+ const itemCount = React.useMemo( + () => Math.max(rows.length, pageSize), + [rows.length, pageSize] + ); return ( <FixedSizeList ... - itemCount={Math.max(rows.length, pageSize)} + itemCount={itemCount} ... >app/client/src/widgets/TableWidgetV2/component/VirtualTable.tsx (1)
40-42: Add JSDoc comments for the new props.The new infinite scroll related props would benefit from documentation explaining their purpose and usage.
+ /** Whether infinite scroll is enabled for the table */ isInfiniteScrollEnabled: boolean; + /** Whether the table is currently loading more data */ isLoading: boolean; + /** Callback to load more data when scrolling */ loadMoreFromEvaluations: () => void;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
app/client/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (13)
app/client/package.json(3 hunks)app/client/src/ce/entities/FeatureFlag.ts(2 hunks)app/client/src/widgets/TableWidgetV2/component/StaticTable.tsx(2 hunks)app/client/src/widgets/TableWidgetV2/component/Table.tsx(5 hunks)app/client/src/widgets/TableWidgetV2/component/TableBody/InifiniteScrollBody/index.tsx(1 hunks)app/client/src/widgets/TableWidgetV2/component/TableBody/InifiniteScrollBody/useInfiniteVirtualization.tsx(1 hunks)app/client/src/widgets/TableWidgetV2/component/TableBody/VirtualList.tsx(1 hunks)app/client/src/widgets/TableWidgetV2/component/TableBody/index.tsx(4 hunks)app/client/src/widgets/TableWidgetV2/component/VirtualTable.tsx(2 hunks)app/client/src/widgets/TableWidgetV2/component/index.tsx(3 hunks)app/client/src/widgets/TableWidgetV2/constants.ts(2 hunks)app/client/src/widgets/TableWidgetV2/widget/index.tsx(1 hunks)app/client/src/widgets/TableWidgetV2/widget/propertyConfig/contentConfig.ts(3 hunks)
👮 Files not reviewed due to content moderation or server errors (4)
- app/client/src/widgets/TableWidgetV2/component/index.tsx
- app/client/src/widgets/TableWidgetV2/widget/propertyConfig/contentConfig.ts
- app/client/src/widgets/TableWidgetV2/component/Table.tsx
- app/client/src/widgets/TableWidgetV2/widget/index.tsx
🔇 Additional comments (9)
app/client/src/widgets/TableWidgetV2/component/StaticTable.tsx (2)
45-46: LGTM! Props added for infinite scroll support.The new props enhance the table's capability to handle loading states and trigger evaluation loading.
86-88: LGTM! Props correctly passed to TableBody.The implementation maintains consistency with the static table's behavior by explicitly setting
isInfiniteScrollEnabledto false.app/client/src/ce/entities/FeatureFlag.ts (1)
55-56: LGTM! Feature flag follows naming convention.The feature flag is correctly implemented with a safe default value of false, allowing for controlled rollout.
Also applies to: 101-101
app/client/src/widgets/TableWidgetV2/component/TableBody/index.tsx (3)
12-14: LGTM! Clean component imports.The imports are well-organized and specific to the new functionality.
62-64: Verify loading state handling.The
isLoadingprop is added but there's no visible implementation of loading indicators in the component tree.Could you share how the loading state is visually represented to users during data fetching?
167-174: LGTM! Clean conditional rendering implementation.The component elegantly handles different rendering modes based on scroll and virtualization settings.
app/client/src/widgets/TableWidgetV2/constants.ts (1)
114-114: LGTM! Clean feature flag integration.The implementation properly integrates with the feature flag system and extends the widget props interface.
Also applies to: 248-249
app/client/package.json (2)
99-99: LGTM! Dependencies added for infinite scroll implementation.The addition of
react-window-infinite-loaderand its type definitions aligns with the PR objective of implementing infinite scroll functionality for the table widget.Also applies to: 210-210
99-99:✅ Verification successful
Verify package versions for security vulnerabilities.
Let's check if these versions have any known security vulnerabilities.
Also applies to: 210-210
🌐 Web query:
Are there any known security vulnerabilities in react-window-infinite-loader version 1.0.10?💡 Result:
Based on the search results reviewed, there are no known security vulnerabilities specifically reported for
react-window-infinite-loaderversion 1.0.10. Here's a breakdown of the findings:Key Points
Snyk Vulnerability Database ([5][8][9][10]) explicitly states:
- "No direct vulnerabilities have been found for this package in Snyk’s vulnerability database."
- This includes versions up to 1.0.10 (as of February 2025).
NPM Registry and GitHub Documentation ([1][3][7]) confirm:
- The library focuses on lazy-loading large datasets and does not handle sensitive operations like network requests or user input processing inherently prone to exploits (e.g., XSS or RCE).
- No security advisories or CVEs are listed for any versions of
react-window-infinite-loader.General React Security Guidance ([4]) highlights common risks like XSS but does not link them to this library:
- Since
react-window-infinite-loaderprimarily manages rendering logic (not data fetching or DOM manipulation), it avoids many attack vectors by design.Recommendations
- Ensure dependencies like
reactandreact-windoware up-to-date, as vulnerabilities in underlying libraries could indirectly affect functionality[4][8].- Follow secure coding practices if extending the library (e.g., sanitizing inputs passed to
loadMoreItems).In summary, version 1.0.10 of
react-window-infinite-loaderhas no publicly reported security issues as of February 2025[5][7][10].Citations:
- 1: https://www.npmjs.com/package/react-window-infinite-loader/v/1.0.3
- 2: https://isomer-user-content.by.gov.sg/36/55d66555-e24b-46bc-b5a4-bf1883ab139d/17-July-2024.pdf
- 3: https://github.com/bvaughn/react-window-infinite-loader
- 4: https://www.loginradius.com/blog/engineering/react-security-vulnerabilities/
- 5: https://security.snyk.io/package/npm/react-window-infinite-loader
- 6: https://github.com/bvaughn/react-window
- 7: https://github.com/bvaughn/react-window-infinite-loader/blob/master/package.json
- 8: https://security.snyk.io/package/npm/react-window
- 9: https://security.snyk.io/package/npm/@types%2Freact-window-infinite-loader/1.0.6
- 10: https://www.npmjs.com/package/react-window-infinite-loader
Security Check Confirmed – No Known Vulnerabilities Detected
The verification confirms that for
react-window-infinite-loaderversion 1.0.10 there are no known security vulnerabilities as per current vulnerability databases. Please ensure the following:
- The type definitions (
@types/react-window-infinite-loaderat version ^1.0.9) remain compatible with the main library.- Verify that related peer dependencies such as
reactandreact-windoware also kept up-to-date.
.../widgets/TableWidgetV2/component/TableBody/InifiniteScrollBody/useInfiniteVirtualization.tsx
Show resolved
Hide resolved
app/client/src/widgets/TableWidgetV2/component/TableBody/InifiniteScrollBody/index.tsx
Show resolved
Hide resolved
Implements comprehensive unit tests for the useInfiniteVirtualization hook, covering scenarios like item loading, loading more items, and handling different loading states for the infinite scroll functionality in the table widget.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/client/src/widgets/TableWidgetV2/component/TableBody/InifiniteScrollBody/useInfiniteVirtualization.test.tsx (2)
7-58: Consider simplifying the mock data structure.The mock rows contain many properties that might not be necessary for testing the infinite scroll functionality. Consider creating a minimal mock structure with only the essential properties needed for these tests.
const mockRows: ReactTableRowType<Record<string, unknown>>[] = [ { id: "1", original: { id: 1, name: "Test 1" }, index: 0, - cells: [], - values: {}, - getRowProps: jest.fn(), - allCells: [], - subRows: [], - isExpanded: false, - canExpand: false, - depth: 0, - toggleRowExpanded: jest.fn(), - state: {}, - toggleRowSelected: jest.fn(), - getToggleRowExpandedProps: jest.fn(), - isSelected: false, - isSomeSelected: false, - isGrouped: false, - groupByID: "", - groupByVal: "", - leafRows: [], - getToggleRowSelectedProps: jest.fn(), - setState: jest.fn(), }, // Similar simplification for the second mock row ];
71-152: Consider adding tests for boundary conditions and error scenarios.The test suite has good coverage but could benefit from additional test cases:
- Boundary conditions for
loadMoreItems(e.g., negative indices, startIndex > stopIndex)- Error handling scenarios (e.g., failed load more operation)
Example test case:
it("should handle loadMore failure gracefully", async () => { const loadMoreWithError = jest.fn().mockRejectedValue(new Error("Load failed")); const { result } = renderHook(() => useInfiniteVirtualization({ ...defaultProps, loadMore: loadMoreWithError, }), ); await act(async () => { await result.current.loadMoreItems(0, 10); }); expect(loadMoreWithError).toHaveBeenCalledTimes(1); // Add assertions for error handling behavior });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/client/src/widgets/TableWidgetV2/component/TableBody/InifiniteScrollBody/useInfiniteVirtualization.test.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-build / client-build
🔇 Additional comments (1)
app/client/src/widgets/TableWidgetV2/component/TableBody/InifiniteScrollBody/useInfiniteVirtualization.test.tsx (1)
60-69: LGTM! Clean test setup.The default props and beforeEach setup are well-structured and follow testing best practices.
Extends the test configuration for TableWidgetV2 to include the infiniteScrollEnabled flag, supporting the recent infinite scroll implementation and test coverage.
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/13307171794. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/client/src/widgets/TableWidgetV2/widget/__tests__/TableRendered.test.ts (1)
113-181: Add test coverage for infinite scroll functionality.The test suite focuses on loading states but lacks coverage for the new infinite scroll feature. Please add test cases to verify:
- Infinite scroll behavior when enabled/disabled
- Interaction with loading states during scroll
- Scroll event handling
Would you like me to help generate the test cases for infinite scroll functionality?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/client/src/widgets/TableWidgetV2/widget/__tests__/TableRendered.test.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-build / client-build
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (1)
app/client/src/widgets/TableWidgetV2/widget/__tests__/TableRendered.test.ts (1)
110-110: LGTM! Property addition aligns with feature implementation.The addition of
infiniteScrollEnabledproperty with default valuefalseis correct.
|
Deploy-Preview-URL: https://ce-39225.dp.appsmith.com |
…e' of https://github.com/appsmithorg/appsmith into rahulbarwal/issue39082/Implement-scroll-down-triger-to-load-next-page
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/DataUtils.java (2)
174-237: Enhance binary data handling with content type validation.The binary data handling has been improved, but consider adding content type validation before processing the data to ensure only supported formats are handled.
if (jsonNode.has("data")) { String binaryData = jsonNode.get("data").asText(); log.debug("Original binary data length: " + binaryData.length()); + + // Validate content type before processing + if (!isSupportedContentType(binaryData)) { + log.warn("Unsupported content type detected"); + return BodyInserters.fromValue(requestBodyObj); + } // Create a ByteArrayInputStream from the binary data ByteArrayInputStream inputStream =
220-226: Consider logging PDF version for debugging.When handling PDF files, it would be helpful to log the PDF version for debugging purposes.
if (binaryData.startsWith("%PDF-")) { StringBuilder header = new StringBuilder(); for (int i = 0; i < Math.min(8, allBytes.length); i++) { header.append((char) allBytes[i]); } - log.debug("PDF header after processing: " + header); + log.debug("PDF header after processing: {} (Version: {})", header, + header.toString().substring(5, header.indexOf('\n'))); }app/client/src/sagas/ActionExecution/PluginActionSaga.ts (1)
279-298: Consider adding file type validation.The
downloadBinaryFilefunction assumes PDF format. Consider making it more generic to handle different file types.-const downloadBinaryFile = (binaryData: string, filename = "download.pdf") => { +const downloadBinaryFile = ( + binaryData: string, + filename: string, + mimeType = "application/pdf" +) => { // Convert binary string to array buffer const bytes = new Uint8Array(binaryData.length); for (let i = 0; i < binaryData.length; i++) { bytes[i] = binaryData.charCodeAt(i); } // Create blob and download - const blob = new Blob([bytes], { type: "application/pdf" }); + const blob = new Blob([bytes], { type: mimeType }); const url = window.URL.createObjectURL(blob);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
app/client/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
app/client/package.json(2 hunks)app/client/src/sagas/ActionExecution/PluginActionSaga.ts(2 hunks)app/client/src/widgets/TableWidgetV2/component/TableBody/index.tsx(3 hunks)app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/DataUtils.java(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/client/package.json
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-build / client-build
- GitHub Check: server-unit-tests / server-unit-tests
- GitHub Check: server-spotless / spotless-check
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (3)
app/client/src/widgets/TableWidgetV2/component/TableBody/index.tsx (3)
63-64: LGTM! New props for infinite scroll functionality.The addition of
isLoadingandloadMoreFromEvaluationsprops enhances the table's functionality by providing loading state and data fetching capabilities.
71-78: LGTM! Improved virtual list implementation.The
FixedVirtualListcomponent is now properly configured with all required props for virtualization.
167-174:❓ Verification inconclusive
Verify error handling in infinite scroll.
The infinite scroll implementation should handle edge cases such as failed data fetches and error states.
🏁 Script executed:
#!/bin/bash # Search for error handling in InfiniteScrollBody component ast-grep --pattern 'class InfiniteScrollBody { $$$ catch($$$) { $$$ } $$$ }'Length of output: 90
Manual verification of error handling in InfiniteScrollBody is needed.
The initial check for try/catch logic in the InfiniteScrollBody component did not return any results. Please verify manually that the component gracefully handles error states—such as failed data fetches—by implementing proper error handling (e.g., try/catch blocks or error boundaries) if not already present.
- Confirm that error handling is implemented within InfiniteScrollBody to catch exceptions or failures.
- If absent, consider adding error handling logic to manage unexpected states.
| // Check if the resolved value is a string and contains PDF-1.4 | ||
| if (typeof value === "string") { | ||
| downloadBinaryFile(value, `debug_${Date.now()}.pdf`); | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for binary data processing.
The binary data processing should include error handling for malformed data.
if (typeof value === "string") {
+ try {
downloadBinaryFile(value, `debug_${Date.now()}.pdf`);
+ } catch (error) {
+ log.error("Failed to process binary data:", error);
+ throw new Error("Failed to process binary data");
+ }
}📝 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.
| // Check if the resolved value is a string and contains PDF-1.4 | |
| if (typeof value === "string") { | |
| downloadBinaryFile(value, `debug_${Date.now()}.pdf`); | |
| } | |
| // Check if the resolved value is a string and contains PDF-1.4 | |
| if (typeof value === "string") { | |
| try { | |
| downloadBinaryFile(value, `debug_${Date.now()}.pdf`); | |
| } catch (error) { | |
| log.error("Failed to process binary data:", error); | |
| throw new Error("Failed to process binary data"); | |
| } | |
| } |
Description
Implements infinite scroll functionality for table widget using react-window-infinite-loader. Introduces new components and hooks to manage virtualized table rendering with dynamic loading of rows.
Fixes #39082
or
Fixes
Issue URLWarning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.Table, @tag.Sanity"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13329193341
Commit: 0c58fcf
Cypress dashboard.
Tags:
@tag.Table, @tag.SanitySpec:
Fri, 14 Feb 2025 13:18:37 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Chores