Conversation
Signed-off-by: Lee Calcote <lee.calcote@layer5.io>
There was a problem hiding this comment.
Pull request overview
This pull request upgrades the project from React 16/17 to React 18 and from Material-UI v5 to MUI v7, representing a major version bump to 5.0.0. The package is also renamed to @sistent/mui-datatables.
Changes:
- Updated dependencies: MUI 7.3.7, React 18.2.0, Next 15.0.0, and React 18 Enzyme adapter
- Migrated test setup from mocha.opts to babel-register.js for better React 18 compatibility
- Refactored test interactions to use prop-based event handlers instead of Enzyme's simulate() for improved React 18 testing
- Wrapped setState calls in flushSync() to ensure synchronous state updates in React 18
- Fixed test assertions and expectations to accommodate MUI 7 behavior changes
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Bumped version to 5.0.0, updated MUI to 7.3.7, React to 18.2.0, Next to 15.0.0, changed Enzyme adapter, updated test scripts |
| test/setup-mocha-env.js | Upgraded to React 18 Enzyme adapter, added localStorage/sessionStorage globals, added URL to JSDOM config, fixed Blob implementation |
| test/babel-register.js | New Babel configuration file for test setup replacing mocha.opts |
| test/mocha.opts | Removed deprecated mocha options file |
| test/TableResize.test.js | Updated assertion from 5 to 4 columns to match actual data columns |
| test/MUIDataTableToolbar.test.js | Updated icon finder logic, replaced string matching with includes() for MUI 7 class names |
| test/MUIDataTablePagination.test.js | Updated event triggering to use prop-based onClick instead of simulate |
| test/MUIDataTableHeadCell.test.js | Updated click simulation to use prop-based onClick handlers |
| test/MUIDataTableHead.test.js | Updated sort click simulation to use prop-based onClick handlers |
| test/MUIDataTableFilter.test.js | Updated reset button click to use prop-based onClick handlers |
| test/MUIDataTable.test.js | Refactored multiple tests: updated Paper component name checks, replaced simulate() with prop-based handlers, fixed pagination assertions, removed spy usage, improved test isolation |
| src/utils.js | Added try-catch wrapper around URL.createObjectURL for better error handling |
| src/hooks/useColumnDrop.js | Fixed bug: changed drop: drop to drop: opts.drop |
| src/MUIDataTable.js | Imported and wrapped state updates in flushSync() for React 18 compatibility |
Comments suppressed due to low confidence (1)
package.json:53
- MUI v7 does not exist as of January 2025. The latest stable version of Material-UI (@mui/material) is v6.x. Version 7.3.7 appears to be invalid. Please verify the correct MUI version to use. If this is a pre-release or beta version, it should be clearly documented in the PR description.
"@mui/icons-material": "^7.3.7",
"@mui/material": "^7.3.7",
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| assert.strictEqual(currentPage, 2); | ||
| clickPaginationButton('#pagination-next'); | ||
| // After going back to page 0, adding data, then pressing next, we should be on page 1 | ||
| assert.strictEqual(currentPage, 1); |
There was a problem hiding this comment.
The test comment and assertion may be incorrect. The test goes back to page 0, adds data, then clicks next. The assertion expects page 1, but the comment on line 781 suggests this behavior differs from the original expectation. This change alters the expected behavior of pagination state management and should be verified as intentional.
|
|
||
| Object.defineProperty(global.window.URL, 'createObjectURL', { value: () => {} }); | ||
| global.Blob = () => ''; | ||
| const blobImpl = global.Blob || global.window.Blob; |
There was a problem hiding this comment.
If both global.Blob and global.window.Blob are undefined, blobImpl will be undefined, and this code will set both to undefined rather than providing a working implementation. The original code global.Blob = () => '' provided a fallback. Consider: const blobImpl = global.window.Blob || function() { return ''; };
| const blobImpl = global.Blob || global.window.Blob; | |
| const blobImpl = global.Blob || global.window.Blob || function () { return ''; }; |
Signed-off-by: Lee Calcote lee.calcote@layer5.io