Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
21 changes: 19 additions & 2 deletions eslint.config.js
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
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.2",
"@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
Collaborator 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
Collaborator

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.13.0",
"@typescript-eslint/parser": "^8.13.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.8",
"@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.14.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-a7bf2bd-20241110",
"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.16.1",
"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
74 changes: 38 additions & 36 deletions src/DataGrid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,8 @@ function DataGrid<R, SR, K extends Key>(
const [isDragging, setDragging] = useState(false);
const [draggedOverRowIdx, setOverRowIdx] = useState<number | undefined>(undefined);
const [scrollToPosition, setScrollToPosition] = useState<PartialPosition | null>(null);
const [shouldFocusCell, setShouldFocusCell] = useState(false);
const [previousRowIdx, setPreviousRowIdx] = useState(-1);

const getColumnWidth = useCallback(
(column: CalculatedColumn<R, SR>) => {
Expand Down Expand Up @@ -338,15 +340,13 @@ function DataGrid<R, SR, K extends Key>(
const [selectedPosition, setSelectedPosition] = useState(
(): SelectCellState | EditCellState<R> => ({ idx: -1, rowIdx: minRowIdx - 1, mode: 'SELECT' })
);
const [prevSelectedPosition, setPrevSelectedPosition] = useState(selectedPosition);

/**
* refs
*/
const prevSelectedPosition = useRef(selectedPosition);
const latestDraggedOverRowIdx = useRef(draggedOverRowIdx);
const lastSelectedRowIdx = useRef(-1);
const focusSinkRef = useRef<HTMLDivElement>(null);
const shouldFocusCellRef = useRef(false);
Comment on lines -345 to -349
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed a few refs


/**
* computed values
Expand Down Expand Up @@ -458,31 +458,50 @@ function DataGrid<R, SR, K extends Key>(
selectCell({ rowIdx: minRowIdx + rowIdx - 1, idx });
});

/**
* callbacks
*/
const setDraggedOverRowIdx = useCallback((rowIdx?: number) => {
setOverRowIdx(rowIdx);
latestDraggedOverRowIdx.current = rowIdx;
}, []);

const focusCellOrCellContent = useCallback(() => {
const cell = getCellToScroll(gridRef.current!);
if (cell === null) return;

scrollIntoView(cell);
// Focus cell content when available instead of the cell itself
const elementToFocus = cell.querySelector<Element & HTMLOrSVGElement>('[tabindex="0"]') ?? cell;
elementToFocus.focus({ preventScroll: true });
}, [gridRef]);

/**
* effects
*/
useLayoutEffect(() => {
if (
!selectedCellIsWithinSelectionBounds ||
isSamePosition(selectedPosition, prevSelectedPosition.current)
isSamePosition(selectedPosition, prevSelectedPosition)
) {
prevSelectedPosition.current = selectedPosition;
setPrevSelectedPosition(selectedPosition);
return;
}

prevSelectedPosition.current = selectedPosition;
setPrevSelectedPosition(selectedPosition);

if (selectedPosition.idx === -1) {
focusSinkRef.current!.focus({ preventScroll: true });
if (focusSinkRef.current !== null && selectedPosition.idx === -1) {
focusSinkRef.current.focus({ preventScroll: true });
scrollIntoView(focusSinkRef.current);
}
});
}, [selectedCellIsWithinSelectionBounds, selectedPosition, prevSelectedPosition]);

useLayoutEffect(() => {
if (!shouldFocusCellRef.current) return;
shouldFocusCellRef.current = false;
focusCellOrCellContent();
});
if (shouldFocusCell) {
setShouldFocusCell(false);
focusCellOrCellContent();
}
}, [shouldFocusCell, focusCellOrCellContent]);

useImperativeHandle(ref, () => ({
element: gridRef.current,
Expand All @@ -499,14 +518,6 @@ function DataGrid<R, SR, K extends Key>(
selectCell
}));

/**
* callbacks
*/
const setDraggedOverRowIdx = useCallback((rowIdx?: number) => {
setOverRowIdx(rowIdx);
latestDraggedOverRowIdx.current = rowIdx;
}, []);

/**
* event handlers
*/
Expand Down Expand Up @@ -536,9 +547,8 @@ function DataGrid<R, SR, K extends Key>(
if (isRowSelectionDisabled?.(row) === true) return;
const newSelectedRows = new Set(selectedRows);
const rowKey = rowKeyGetter(row);
const previousRowIdx = lastSelectedRowIdx.current;
const rowIdx = rows.indexOf(row);
lastSelectedRowIdx.current = rowIdx;
setPreviousRowIdx(rowIdx);

if (checked) {
newSelectedRows.add(rowKey);
Expand Down Expand Up @@ -758,7 +768,7 @@ function DataGrid<R, SR, K extends Key>(
// Avoid re-renders if the selected cell state is the same
scrollIntoView(getCellToScroll(gridRef.current!));
} else {
shouldFocusCellRef.current = true;
setShouldFocusCell(true);
setSelectedPosition({ ...position, mode: 'SELECT' });
}

Expand Down Expand Up @@ -870,16 +880,6 @@ function DataGrid<R, SR, K extends Key>(
return isDraggedOver ? selectedPosition.idx : undefined;
}

function focusCellOrCellContent() {
const cell = getCellToScroll(gridRef.current!);
if (cell === null) return;

scrollIntoView(cell);
// Focus cell content when available instead of the cell itself
const elementToFocus = cell.querySelector<Element & HTMLOrSVGElement>('[tabindex="0"]') ?? cell;
elementToFocus.focus({ preventScroll: true });
}

function renderDragHandle() {
if (
onFill == null ||
Expand Down Expand Up @@ -925,7 +925,7 @@ function DataGrid<R, SR, K extends Key>(
const colSpan = getColSpan(column, lastFrozenColumnIndex, { type: 'ROW', row });

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

Expand Down Expand Up @@ -1061,6 +1061,7 @@ 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' });
// eslint-disable-next-line react-compiler/react-compiler
setDraggedOverRowIdx(undefined);
Copy link
Collaborator 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
Collaborator

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
Collaborator 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

Copy link
Collaborator

Choose a reason for hiding this comment

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

Mmh, setting ref during render?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, need a workaround. We need ref to access the latest value. I will investigate

}

Expand Down Expand Up @@ -1182,6 +1183,7 @@ function DataGrid<R, SR, K extends Key>(
);
})}
<RowSelectionChangeProvider value={selectRowLatest}>
{/* eslint-disable-next-line react-compiler/react-compiler */}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Don't know which line it is complaining about

Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably the setDraggedOverRowIdx call?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried removing it and it was still showing the same warning

{getViewportRows()}
</RowSelectionChangeProvider>
{bottomSummaryRows?.map((row, rowIdx) => {
Expand Down Expand Up @@ -1245,7 +1247,7 @@ function DataGrid<R, SR, K extends Key>(
<ScrollToCell
scrollToPosition={scrollToPosition}
setScrollToCellPosition={setScrollToPosition}
gridElement={gridRef.current!}
gridRef={gridRef}
Copy link
Collaborator 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
Collaborator 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
Collaborator

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
3 changes: 1 addition & 2 deletions tsconfig.website.json
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
Loading