Conversation
Signed-off-by: Lee Calcote <lee.calcote@layer5.io>
There was a problem hiding this comment.
Pull request overview
This PR upgrades a broad set of tooling/runtime packages and applies accompanying code/test adjustments (mostly formatting and small API updates) to keep the project building and tests running with the newer dependency versions.
Changes:
- Upgraded build/test/tooling dependencies (Babel/Rollup/Webpack/Mocha/Prettier/etc.) and related configuration.
- Updated various components/hooks/tests for updated ecosystem APIs and formatting (notably
react-dndhook usage and jsdom/mocha environment setup). - Removed
lodash.isequalusage and adjusted filter equality logic inMUIDataTable.
Reviewed changes
Copilot reviewed 45 out of 52 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/setup-mocha-env.js | Updates jsdom/mocha globals (notably navigator) to better match newer jsdom behavior. |
| test/UseColumnDrop.test.js | Formatting-only updates to match newer lint/prettier output. |
| test/TableResize.test.js | Formatting-only updates and minor refactor of chained calls. |
| test/MUIDataTableViewCol.test.js | Formatting-only updates (arrow function parentheses). |
| test/MUIDataTableToolbarSelect.test.js | Formatting-only updates (arrow function parentheses). |
| test/MUIDataTableToolbarCustomIcons.test.js | Formatting-only updates (arrow function parentheses). |
| test/MUIDataTableToolbar.test.js | Formatting-only updates (arrow function parentheses). |
| test/MUIDataTableSelectCell.test.js | Formatting-only updates (arrow function parentheses). |
| test/MUIDataTableSearch.test.js | Formatting-only updates (arrow function parentheses). |
| test/MUIDataTablePagination.test.js | Formatting-only updates (arrow function parentheses). |
| test/MUIDataTableHeadCell.test.js | Formatting-only updates and minor JSX formatting changes. |
| test/MUIDataTableHead.test.js | Formatting-only updates (arrow function parentheses). |
| test/MUIDataTableFooter.test.js | Formatting-only updates (arrow function parentheses). |
| test/MUIDataTableFilterList.test.js | Formatting-only updates and minor chaining refactors. |
| test/MUIDataTableFilter.test.js | Formatting updates, but retains a broken map usage that reassigns columns incorrectly. |
| test/MUIDataTableCustomComponents.test.js | Formatting-only updates (arrow function parentheses). |
| test/MUIDataTableBodyCell.test.js | Formatting-only updates and minor chaining refactors. |
| test/MUIDataTableBody.test.js | Formatting-only updates and minor chaining refactors. |
| test/MUIDataTable.test.js | Formatting-only updates and minor chaining refactors. |
| src/utils.js | Formatting-only updates (arrow function parentheses). |
| src/plug-ins/DebounceSearchRender.js | Formatting-only updates (arrow function parentheses) for debounce/search wiring. |
| src/localStorage/load.js | Formatting-only update (arrow function parentheses). |
| src/hooks/useColumnDrop.js | Formatting-only updates (arrow function parentheses) for DnD hook helpers. |
| src/components/TableViewCol.js | Updates tss-react makeStyles call signature formatting and handler formatting. |
| src/components/TableToolbarSelect.js | Formatting-only updates (arrow function parentheses). |
| src/components/TableToolbar.js | Formatting-only updates and minor JSX formatting. |
| src/components/TableSelectCell.js | Updates tss-react makeStyles call signature and ref callback formatting. |
| src/components/TableSearch.js | Updates tss-react makeStyles call signature and handler formatting. |
| src/components/TableResize.js | Formatting-only updates (arrow function parentheses) and JSX formatting. |
| src/components/TablePagination.js | Updates tss-react makeStyles call signature and handler formatting. |
| src/components/TableHeadRow.js | JSX formatting-only change. |
| src/components/TableHeadCell.js | Updates tss-react styles signature and migrates react-dnd useDrag spec usage. |
| src/components/TableHead.js | Updates tss-react styles signature and JSX formatting. |
| src/components/TableFilterList.js | Formatting-only updates (arrow function parentheses). |
| src/components/TableFilter.js | Formatting-only updates (arrow function parentheses) and JSX formatting. |
| src/components/TableBodyRow.js | JSX formatting-only change. |
| src/components/TableBodyCell.js | Updates tss-react styles signature and minor formatting refactor. |
| src/components/TableBody.js | Formatting-only updates and JSX formatting. |
| src/components/Popover.js | Formatting-only updates and JSX formatting. |
| src/components/JumpToPage.js | Updates tss-react styles signature, handler formatting, and JSX formatting. |
| src/components/ExpandButton.js | JSX formatting-only change. |
| src/MUIDataTable.js | Removes lodash.isequal and changes filter equality logic; broad formatting updates. |
| rollup.config.js | Adds preventAssignment for newer @rollup/plugin-replace behavior. |
| package.json | Major dependency upgrades (tooling + runtime/dev deps). |
| docs/utils/withRoot.js | Formatting-only updates (arrow function parentheses). |
| docs/utils/layout.js | Formatting-only updates and JSX formatting. |
| docs/utils/Menu.js | Formatting-only updates and JSX formatting. |
| docs/utils/CodeSnippet.js | Formatting-only updates (arrow function parentheses). |
| docs/pages/index.js | Formatting-only updates (arrow function parentheses). |
| docs/pages/_document.js | Formatting-only updates (arrow function parentheses). |
| .npmrc | Adds legacy-peer-deps=true affecting installation behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| updateFilterByType = (filterList, index, value, type, customUpdate) => { | ||
| const filterPos = filterList[index].findIndex(filter => isEqual(filter, value)); | ||
| const filterPos = filterList[index].findIndex((filter) => JSON.stringify(filter) === JSON.stringify(value)); | ||
|
|
There was a problem hiding this comment.
JSON.stringify-based equality is not a safe replacement for deep equality here: it treats functions as undefined, can throw on circular values, and can mis-compare objects when key insertion order differs. This can break filter toggling/removal for non-primitive filter values (e.g., custom filter values that are objects/arrays). Consider restoring a real deep-equality check (e.g., reintroduce lodash.isequal/fast-deep-equal), or constrain filter values to primitives and compare accordingly.
| const options = { filterType: 'checkbox', textLabels: getTextLabels() }; | ||
| const filterList = [[], [], [], []]; | ||
| columns = columns.map(item => (item.filter = false)); | ||
| columns = columns.map((item) => (item.filter = false)); |
There was a problem hiding this comment.
This mutates each column and then replaces columns with the result of the assignment expression (i.e., an array of false booleans). That will break <TableFilter /> rendering since it expects column objects. Use columns = columns.map((item) => ({ ...item, filter: false })) or mutate with forEach without reassigning columns.
| const options = { filterType: 'select', textLabels: getTextLabels() }; | ||
| const filterList = [['Joe James'], [], [], []]; | ||
| columns = columns.map(item => (item.filter = false)); | ||
| columns = columns.map((item) => (item.filter = false)); |
There was a problem hiding this comment.
This map call reassigns columns to an array of false values (because (item.filter = false) returns false), which will cause <TableFilter /> to crash when it tries to read properties like column.filter. Use a mapping that returns the column object (or a forEach mutation) instead.
| const options = { filterType: 'textField', textLabels: getTextLabels() }; | ||
| const filterList = [[], [], [], []]; | ||
| columns = columns.map(item => (item.filter = false)); | ||
| columns = columns.map((item) => (item.filter = false)); |
There was a problem hiding this comment.
Same issue as above: this map expression turns columns into an array of booleans, not column objects, which can break the test/component rendering. Return the mutated column object (or create a new one) instead of returning the assignment result.
| "react-dnd-test-backend": "^16.0.1", | ||
| "react-dnd-test-utils": "^16.0.1", | ||
| "react-dom": "^18.3.1", | ||
| "react-router-dom": "^7.13.2", |
There was a problem hiding this comment.
react-router-dom is bumped to v7, but the examples currently import/use APIs removed after v5 (e.g., Switch, withRouter, Route component=). This upgrade will break the examples build/runtime unless those files are migrated to the v6+ API (Routes, useNavigate, etc.) or the dependency is kept on v5.
| "react-router-dom": "^7.13.2", | |
| "react-router-dom": "^5.3.4", |
| @@ -0,0 +1 @@ | |||
| legacy-peer-deps=true | |||
There was a problem hiding this comment.
Adding legacy-peer-deps=true forces npm to ignore peer dependency conflicts, which can mask real compatibility issues during development/CI and make dependency problems harder to diagnose. If this is only needed to unblock a specific upgrade, consider scoping it to CI (env var) or documenting why it's required so it can be removed later.
| legacy-peer-deps=true |
No description provided.