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

[DataGrid] Add slot typings #11795

Merged
merged 17 commits into from Feb 5, 2024
Merged

[DataGrid] Add slot typings #11795

merged 17 commits into from Feb 5, 2024

Conversation

romgrk
Copy link
Contributor

@romgrk romgrk commented Jan 23, 2024

Closes #11687

@romgrk romgrk added breaking change component: data grid This is the name of the generic UI component, not the React module! labels Jan 23, 2024
@mui-bot
Copy link

mui-bot commented Jan 23, 2024

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 24, 2024
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 24, 2024
@@ -49,44 +56,52 @@ export interface LoadingOverlayPropsOverrides {}
export interface NoResultsOverlayPropsOverrides {}
export interface NoRowsOverlayPropsOverrides {}
export interface PanelPropsOverrides {}
export interface PinnedRowsPropsOverrides {}
export interface SkeletonCellPropsOverrides {}
export interface RowPropsOverrides {}
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 don't love these but I've added the missing ones for consistency.

ref={handleRef}
ref={handleRef as any /* FIXME: typing error */}
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 fixed the straightfoward typing errors, but left FIXME comments for the ones that I couldn't find an easy answer to (mainly because the typings are in MUI core's codebase, so they're harder to fix). For the baseCheckbox refs, the error is often a type mismatch between HTMLInputElement vs HTMLButtonElement which seemed harmless.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sidenote but while working on this I realized that making the datagrid work without MUI is going to be painful. Most of our base slot typings come directly from MUI, so we might have to duplicate typings. Or make users install MUI locally even when it's not bundled.

Copy link
Member

Choose a reason for hiding this comment

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

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 25, 2024
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 25, 2024
@@ -8,5 +8,7 @@ export function GridFooterPlaceholder() {
return null;
}

return <rootProps.slots.footer {...rootProps.slotProps?.footer} />;
return (
<rootProps.slots.footer {...(rootProps.slotProps?.footer as any) /* FIXME: typing error */} />
Copy link
Member

Choose a reason for hiding this comment

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

This one is interesting.
The reason it's failing without as any is this override in the docs examples:

declare module '@mui/x-data-grid' {
interface FooterPropsOverrides {
status: FooterStatus;
}
}

I there something we can do to isolate overrides in docs from the packages?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The way it's setup, the docs is importing the grid through relative imports (behind aliased paths). If the docs were importing code as regular packages, then the skipLibCheck flag would do that iiuc, but it doesn't apply here since it's just regular files. I think it could probably be done, but I'm not sure how long it will take to setup, so I'd skip it for this PR.

@romgrk
Copy link
Contributor Author

romgrk commented Jan 30, 2024

Do we want to take the time to fix the remaining typing errors or are we ok with this change as it is?

Copy link
Member

@cherniavskii cherniavskii left a comment

Choose a reason for hiding this comment

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

Could you add a guide for typing a custom slot and mention the change in the migration guide?

@romgrk romgrk enabled auto-merge (squash) February 5, 2024 15:02
@romgrk romgrk merged commit c5d5a17 into mui:next Feb 5, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change component: data grid This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[datagrid] Add typing to slot props
4 participants