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.

Sign up for GitHub

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

Jump to bottom

Update eslint deps #3615

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Update eslint deps #3615

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
1ae3d7f
Update deps
amanmahajan7 Oct 8, 2024
902d96d
Add new rule
amanmahajan7 Oct 8, 2024
2141ff4
Add `@tanstack/eslint-plugin-router`
amanmahajan7 Oct 8, 2024
5878ec9
Merge branch 'main' into am-eslint
amanmahajan7 Oct 9, 2024
6fcc3fc
Merge branch 'main' into am-eslint
amanmahajan7 Oct 17, 2024
3245e76
Update router deps
amanmahajan7 Oct 17, 2024
0680367
update deps
amanmahajan7 Oct 24, 2024
ca70f3b
Add `'@typescript-eslint/no-unnecessary-type-parameters': 1,`
amanmahajan7 Oct 24, 2024
61a661e
Merge branch 'main' into am-eslint
amanmahajan7 Oct 24, 2024
d6eb0dd
Remove @tanstack/eslint-plugin-router
amanmahajan7 Oct 24, 2024
c5b8f74
Add `eslint-plugin-react-hooks-extra`
amanmahajan7 Oct 24, 2024
706019a
1 more
amanmahajan7 Oct 24, 2024
8438db8
Add `eslint-plugin-react-compiler`
amanmahajan7 Oct 24, 2024
a3d1f21
-1 array
amanmahajan7 Oct 24, 2024
9ca5f15
Do we need these rules
amanmahajan7 Oct 24, 2024
5ce5789
disable 1 `skipLibCheck`, add back newline
nstepien Oct 25, 2024
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
21 changes: 19 additions & 2 deletions eslint.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ 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';

Expand All @@ -18,7 +20,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 +375,22 @@ 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/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 +482,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 +603,7 @@ export default [
plugins: {
vitest,
'jest-dom': jestDom,
'testing-library': fixupPluginRules(testingLibrary)
'testing-library': testingLibrary
},

rules: {
Expand Down Expand Up @@ -647,6 +663,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
View file
Open in desktop
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
View file
Open in desktop
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.

facebook/react#31358

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?

}

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 •
edited
Loading

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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
3 changes: 1 addition & 2 deletions tsconfig.website.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
{
"extends": "./tsconfig.base.json",
"compilerOptions": {
"lib": ["ESNext", "DOM", "DOM.Iterable", "DOM.AsyncIterable"],
"skipLibCheck": true
"lib": ["ESNext", "DOM", "DOM.Iterable", "DOM.AsyncIterable"]
},
"include": ["website/**/*"],
"references": [{ "path": "tsconfig.src.json" }]
Expand Down
77 changes: 36 additions & 41 deletions website/routes/ColumnSpanning.lazy.tsx
View file
Open in desktop
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
View file
Open in desktop
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