Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update eslint deps #3615

Merged
merged 20 commits into from
Nov 11, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions eslint.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ import tsParser from '@typescript-eslint/parser';
import vitest from '@vitest/eslint-plugin';
import jestDom from 'eslint-plugin-jest-dom';
import react from 'eslint-plugin-react';
import reactCompiler from 'eslint-plugin-react-compiler';
import reactHooks from 'eslint-plugin-react-hooks';
import reactHooksExtra from 'eslint-plugin-react-hooks-extra';
import sonarjs from 'eslint-plugin-sonarjs';
import testingLibrary from 'eslint-plugin-testing-library';

export default [
{
ignores: ['.cache', 'coverage', 'dist', 'lib']
Expand All @@ -18,7 +19,9 @@ export default [

plugins: {
react,
'react-compiler': reactCompiler,
'react-hooks': fixupPluginRules(reactHooks),
'react-hooks-extra': reactHooksExtra,
sonarjs,
'@typescript-eslint': typescriptEslint
},
Expand Down Expand Up @@ -371,11 +374,24 @@ export default [
'react/style-prop-object': 0,
'react/void-dom-elements-no-children': 1,

// React Compiler
// https://react.dev/learn/react-compiler#installing-eslint-plugin-react-compiler
'react-compiler/react-compiler': 1,

// React Hooks
// https://www.npmjs.com/package/eslint-plugin-react-hooks
'react-hooks/rules-of-hooks': 1,
'react-hooks/exhaustive-deps': 1,

// React Hooks Extra
// https://eslint-react.xyz/
'react-hooks-extra/no-redundant-custom-hook': 1,
'react-hooks-extra/no-unnecessary-use-callback': 1,
'react-hooks-extra/no-unnecessary-use-memo': 1,
'react-hooks-extra/no-direct-set-state-in-use-effect': 1,
'react-hooks-extra/no-direct-set-state-in-use-layout-effect': 1,
'react-hooks-extra/prefer-use-state-lazy-initialization': 1,

// SonarJS rules
// https://github.com/SonarSource/eslint-plugin-sonarjs#rules
'sonarjs/no-all-duplicated-branches': 1,
Expand Down Expand Up @@ -467,13 +483,14 @@ export default [
'@typescript-eslint/no-this-alias': 0,
'@typescript-eslint/no-type-alias': 0,
'@typescript-eslint/no-unnecessary-boolean-literal-compare': 1,
'@typescript-eslint/no-unnecessary-condition': 1,
'@typescript-eslint/no-unnecessary-condition': [1, { checkTypePredicates: true }],
'@typescript-eslint/no-unnecessary-parameter-property-assignment': 1,
'@typescript-eslint/no-unnecessary-qualifier': 0,
'@typescript-eslint/no-unnecessary-template-expression': 1,
'@typescript-eslint/no-unnecessary-type-arguments': 1,
'@typescript-eslint/no-unnecessary-type-assertion': 1,
'@typescript-eslint/no-unnecessary-type-constraint': 1,
'@typescript-eslint/no-unnecessary-type-parameters': 1,
'@typescript-eslint/no-unsafe-argument': 0,
'@typescript-eslint/no-unsafe-assignment': 0,
'@typescript-eslint/no-unsafe-call': 0,
Expand Down Expand Up @@ -587,7 +604,7 @@ export default [
plugins: {
vitest,
'jest-dom': jestDom,
'testing-library': fixupPluginRules(testingLibrary)
'testing-library': testingLibrary
},

rules: {
Expand Down Expand Up @@ -647,6 +664,7 @@ export default [
'vitest/prefer-to-contain': 1,
'vitest/prefer-to-have-length': 1,
'vitest/prefer-todo': 1,
'vitest/prefer-vi-mocked': 1,
'vitest/require-hook': 0,
'vitest/require-local-test-context-for-concurrent-snapshots': 0,
'vitest/require-to-throw-message': 0,
Expand Down
22 changes: 12 additions & 10 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,37 +62,39 @@
"@babel/preset-typescript": "^7.18.6",
"@babel/runtime": "^7.21.5",
"@biomejs/biome": "1.9.4",
"@eslint/compat": "^1.1.1",
"@eslint/compat": "^1.2.1",
"@faker-js/faker": "^9.0.0",
"@ianvs/prettier-plugin-sort-imports": "^4.0.2",
"@linaria/core": "^6.0.0",
"@microsoft/api-extractor": "^7.23.0",
"@rollup/plugin-babel": "^6.0.3",
"@rollup/plugin-node-resolve": "^15.1.0",
"@tanstack/react-router": "^1.57.13",
"@tanstack/router-plugin": "^1.57.13",
"@tanstack/react-router": "^1.70.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the eslint-plugin. Not sure why is it failing

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the plugin needs to export Plugin, manually tweaking the declaration files fixes it.
Composite projects require declaration files to be emitted, and TS can't emit a declaration file without access to the Plugin type from the @tanstack/eslint-plugin-router.
Should open an issue.

"@tanstack/router-plugin": "^1.69.1",
"@testing-library/dom": "^10.1.0",
"@testing-library/react": "^16.0.0",
"@testing-library/user-event": "^14.5.2",
"@types/node": "^22.0.0",
"@types/react": "^18.3.9",
"@types/react-dom": "^18.3.0",
"@typescript-eslint/eslint-plugin": "^8.7.0",
"@typescript-eslint/parser": "^8.7.0",
"@typescript-eslint/eslint-plugin": "^8.11.0",
"@typescript-eslint/parser": "^8.11.0",
"@vitejs/plugin-react": "^4.3.1",
"@vitest/browser": "^2.1.1",
"@vitest/coverage-v8": "^2.1.1",
"@vitest/eslint-plugin": "^1.1.4",
"@vitest/eslint-plugin": "^1.1.7",
"@wyw-in-js/rollup": "^0.5.0",
"@wyw-in-js/vite": "^0.5.0",
"babel-plugin-optimize-clsx": "^2.6.2",
"browserslist": "^4.24.0",
"eslint": "^9.11.1",
"eslint": "^9.13.0",
"eslint-plugin-jest-dom": "^5.0.1",
"eslint-plugin-react": "^7.36.1",
"eslint-plugin-react": "^7.37.2",
"eslint-plugin-react-compiler": "^19.0.0-beta-8a03594-20241020",
"eslint-plugin-react-hooks": "^5.0.0",
"eslint-plugin-sonarjs": "^2.0.2",
"eslint-plugin-testing-library": "^6.3.0",
"eslint-plugin-react-hooks-extra": "^1.15.0",
"eslint-plugin-sonarjs": "^2.0.4",
"eslint-plugin-testing-library": "^6.4.0",
"jspdf": "^2.5.1",
"jspdf-autotable": "^3.5.23",
"playwright": "^1.45.1",
Expand Down
19 changes: 11 additions & 8 deletions src/DataGrid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -917,18 +917,22 @@ function DataGrid<R, SR, K extends Key>(
);
}

function cancelEditing() {
setSelectedPosition(({ idx, rowIdx }) => ({ idx, rowIdx, mode: 'SELECT' }));
}

function closeEditor(shouldFocusCell: boolean) {
shouldFocusCellRef.current = shouldFocusCell;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we just use a state instead of dancing around the limitations?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can completely remove the ref and use flushSync. It works but it seems like the checkbox checked state is incorrect when cell is clicked. Maybe a bug in React

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a state now

cancelEditing();
}

function getCellEditor(rowIdx: number) {
if (selectedPosition.rowIdx !== rowIdx || selectedPosition.mode === 'SELECT') return;

const { idx, row } = selectedPosition;
const column = columns[idx];
const colSpan = getColSpan(column, lastFrozenColumnIndex, { type: 'ROW', row });

const closeEditor = (shouldFocusCell: boolean) => {
shouldFocusCellRef.current = shouldFocusCell;
setSelectedPosition(({ idx, rowIdx }) => ({ idx, rowIdx, mode: 'SELECT' }));
};

const onRowChange = (row: R, commitChanges: boolean, shouldFocusCell: boolean) => {
if (commitChanges) {
// Prevents two issues when editor is closed by clicking on a different cell
Expand All @@ -946,7 +950,7 @@ function DataGrid<R, SR, K extends Key>(

if (rows[selectedPosition.rowIdx] !== selectedPosition.originalRow) {
// Discard changes if rows are updated from outside
closeEditor(false);
cancelEditing();
}

return (
Expand Down Expand Up @@ -1061,7 +1065,6 @@ function DataGrid<R, SR, K extends Key>(
// Reset the positions if the current values are no longer valid. This can happen if a column or row is removed
if (selectedPosition.idx > maxColIdx || selectedPosition.rowIdx > maxRowIdx) {
setSelectedPosition({ idx: -1, rowIdx: minRowIdx - 1, mode: 'SELECT' });
setDraggedOverRowIdx(undefined);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed? We do set it to undefined after the drag operation
https://github.com/adazzle/react-data-grid/blob/main/src/DragHandle.tsx#L103

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible that the rows update while we're dragging?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably. I added it back for now. I will look into alternatives

}

let templateRows = `repeat(${headerRowsCount}, ${headerRowHeight}px)`;
Expand Down Expand Up @@ -1245,7 +1248,7 @@ function DataGrid<R, SR, K extends Key>(
<ScrollToCell
scrollToPosition={scrollToPosition}
setScrollToCellPosition={setScrollToPosition}
gridElement={gridRef.current!}
gridRef={gridRef}
Copy link
Contributor Author

@amanmahajan7 amanmahajan7 Oct 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eslint-plugin did not catch this. Maybe because it is conditional 🤔

/>
)}
</div>
Expand Down
8 changes: 4 additions & 4 deletions src/ScrollToCell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ export interface PartialPosition {

export default function ScrollToCell({
scrollToPosition: { idx, rowIdx },
gridElement,
gridRef,
setScrollToCellPosition
}: {
scrollToPosition: PartialPosition;
gridElement: HTMLDivElement;
gridRef: React.RefObject<HTMLDivElement | null>;
setScrollToCellPosition: (cell: null) => void;
}) {
const ref = useRef<HTMLDivElement>(null);
Expand All @@ -31,7 +31,7 @@ export default function ScrollToCell({
}

const observer = new IntersectionObserver(removeScrollToCell, {
root: gridElement,
root: gridRef.current!,
threshold: 1.0
});

Expand All @@ -40,7 +40,7 @@ export default function ScrollToCell({
return () => {
observer.disconnect();
};
}, [gridElement, setScrollToCellPosition]);
}, [gridRef, setScrollToCellPosition]);

return (
<div
Expand Down
1 change: 1 addition & 0 deletions src/hooks/useViewportColumns.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export function useViewportColumns<R, SR>({

const updateStartIdx = (colIdx: number, colSpan: number | undefined) => {
if (colSpan !== undefined && colIdx + colSpan > colOverscanStartIdx) {
// eslint-disable-next-line react-compiler/react-compiler
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what the issue is
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should open an issue

startIdx = colIdx;
return true;
}
Expand Down
4 changes: 2 additions & 2 deletions src/utils/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { CalculatedColumn, CalculatedColumnOrColumnGroup } from '../types';
import type { CalculatedColumn, CalculatedColumnOrColumnGroup, Maybe } from '../types';

export * from './colSpanUtils';
export * from './domUtils';
Expand All @@ -11,7 +11,7 @@ export * from './styleUtils';
export const { min, max, floor, sign, abs } = Math;

export function assertIsValidKeyGetter<R, K extends React.Key>(
keyGetter: unknown
keyGetter: Maybe<(row: NoInfer<R>) => K>
): asserts keyGetter is (row: R) => K {
if (typeof keyGetter !== 'function') {
throw new Error('Please specify the rowKeyGetter prop to use selection');
Expand Down
77 changes: 36 additions & 41 deletions website/routes/ColumnSpanning.lazy.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { useMemo } from 'react';
import { createLazyFileRoute } from '@tanstack/react-router';
import { css } from '@linaria/core';

Expand All @@ -20,49 +19,45 @@ const colSpanClassname = css`
text-align: center;
`;

function ColumnSpanning() {
const direction = useDirection();

const columns = useMemo((): readonly Column<Row>[] => {
const columns: Column<Row>[] = [];
const columns: Column<Row>[] = [];

for (let i = 0; i < 30; i++) {
const key = String(i);
columns.push({
key,
name: key,
frozen: i < 5,
resizable: true,
renderCell: renderCoordinates,
colSpan(args) {
if (args.type === 'ROW') {
if (key === '2' && args.row === 2) return 3;
if (key === '4' && args.row === 4) return 6; // Will not work as colspan includes both frozen and regular columns
if (key === '0' && args.row === 5) return 5;
if (key === '27' && args.row === 8) return 3;
if (key === '6' && args.row < 8) return 2;
}
if (args.type === 'HEADER' && key === '8') {
return 3;
}
return undefined;
},
cellClass(row) {
if (
(key === '0' && row === 5) ||
(key === '2' && row === 2) ||
(key === '27' && row === 8) ||
(key === '6' && row < 8)
) {
return colSpanClassname;
}
return undefined;
}
});
for (let i = 0; i < 30; i++) {
const key = String(i);
columns.push({
key,
name: key,
frozen: i < 5,
resizable: true,
renderCell: renderCoordinates,
colSpan(args) {
if (args.type === 'ROW') {
if (key === '2' && args.row === 2) return 3;
if (key === '4' && args.row === 4) return 6; // Will not work as colspan includes both frozen and regular columns
if (key === '0' && args.row === 5) return 5;
if (key === '27' && args.row === 8) return 3;
if (key === '6' && args.row < 8) return 2;
}
if (args.type === 'HEADER' && key === '8') {
return 3;
}
return undefined;
},
cellClass(row) {
if (
(key === '0' && row === 5) ||
(key === '2' && row === 2) ||
(key === '27' && row === 8) ||
(key === '6' && row < 8)
) {
return colSpanClassname;
}
return undefined;
}
});
}

return columns;
}, []);
function ColumnSpanning() {
const direction = useDirection();

return (
<DataGrid
Expand Down
17 changes: 10 additions & 7 deletions website/routes/CommonFeatures.lazy.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -246,17 +246,23 @@ function rowKeyGetter(row: Row) {
return row.id;
}

let countries: string[] = [];

function createRows(): readonly Row[] {
const now = Date.now();
const rows: Row[] = [];
const countrySet = new Set<string>();

for (let i = 0; i < 1000; i++) {
const country = faker.location.country();
countrySet.add(country);

rows.push({
id: i,
title: `Task #${i + 1}`,
client: faker.company.name(),
area: faker.person.jobArea(),
country: faker.location.country(),
country,
contact: faker.internet.exampleEmail(),
assignee: faker.person.fullName(),
progress: Math.random() * 100,
Expand All @@ -270,6 +276,8 @@ function createRows(): readonly Row[] {
});
}

countries = [...countrySet].sort(new Intl.Collator().compare);

return rows;
}

Expand Down Expand Up @@ -313,12 +321,7 @@ function CommonFeatures() {
const [selectedRows, setSelectedRows] = useState((): ReadonlySet<number> => new Set());
const [isExporting, setIsExporting] = useState(false);
const gridRef = useRef<DataGridHandle>(null);

const countries = useMemo((): readonly string[] => {
return [...new Set(rows.map((r) => r.country))].sort(new Intl.Collator().compare);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
const columns = useMemo(() => getColumns(countries, direction), [countries, direction]);
const columns = useMemo(() => getColumns(countries, direction), [direction]);

const summaryRows = useMemo((): readonly SummaryRow[] => {
return [
Expand Down
Loading