Skip to content

Conversation

romgrk
Copy link
Contributor

@romgrk romgrk commented Jul 30, 2025

Remove grid-specific references from the virtualizer API.

Changes

  • Dimensions: Added fields topPinnedHeight and bottomPinnedHeight to the virtualizer (by analogy to leftPinnedWidth and rightPinnedWidth), which the grid fills with the height of the various headers.
  • Remove grid-specific aspects from the virtualizer API
  • Make most column options optional (in preparation for list-like virtualization support)

@romgrk romgrk added internal Behind-the-scenes enhancement. Formerly called “core”. type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature. labels Jul 30, 2025
@mui-bot
Copy link

mui-bot commented Jul 30, 2025

Deploy preview: https://deploy-preview-18995--material-ui-x.netlify.app/

Bundle size report

Bundle Parsed Size Gzip Size
@mui/x-data-grid ▼-633B(-0.16%) ▼-111B(-0.10%)
@mui/x-data-grid-pro ▼-645B(-0.14%) ▼-155B(-0.11%)
@mui/x-data-grid-premium ▼-641B(-0.11%) ▼-164B(-0.10%)
@mui/x-charts 0B(0.00%) 0B(0.00%)
@mui/x-charts-pro 0B(0.00%) 0B(0.00%)
@mui/x-date-pickers 0B(0.00%) 0B(0.00%)
@mui/x-date-pickers-pro 0B(0.00%) 0B(0.00%)
@mui/x-tree-view 0B(0.00%) 0B(0.00%)
@mui/x-tree-view-pro 0B(0.00%) 0B(0.00%)

Details of bundle changes

Generated by 🚫 dangerJS against 88c2f69

@romgrk romgrk marked this pull request as draft July 30, 2025 22:31
@romgrk romgrk marked this pull request as ready for review August 5, 2025 16:05
@@ -26,11 +26,17 @@ export type PinnedRows = {
top: RowEntry[];
bottom: RowEntry[];
};
export namespace PinnedRows {
Copy link
Member

@Janpot Janpot Aug 5, 2025

Choose a reason for hiding this comment

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

Codesandbox bundling seems to just drop the initializer altogether. Prior history of causing issues. Generally declared as a feature to avoid because it confuses tooling all around. While I understand the appeal of having a name refer to both a type and a value at the same time, because of aforementioned reasons I would like to ban all usage of the namespace feature.

How do you feel about any of the following patterns:

export type PinnedRows = {
  top: RowEntry[];
  bottom: RowEntry[];
};
export const PINNED_ROWS = {
  empty: { top: [], bottom: [] } as PinnedRows
}
// or
export const PINNED_ROWS = {
  EMPTY: { top: [], bottom: [] } as PinnedRows
}
// or
export const PinnedRowsValues = {
  EMPTY: { top: [], bottom: [] } as PinnedRows
}
// or
export const PINNED_ROWS_EMPTY: PinnedRows = { top: [], bottom: [] };
export type PinnedRowsType = {
  top: RowEntry[];
  bottom: RowEntry[];
};

export const PinnedRows = {
  EMPTY: { top: [], bottom: [] } as PinnedRowsType
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So there is no way to export values and types on the same identifier without modules?

Copy link
Member

Choose a reason for hiding this comment

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

not that I'm aware of.

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 it just the namespace keyword that's buggy, or using the same identifier? This works for typescript, because types and values live in separate namespaces:

export type PinnedRows = {
  top: RowEntry[];
  bottom: RowEntry[];
};
export const PinnedRows = {
  EMPTY: { top: [], bottom: [] } as PinnedRows,
};

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.

LGTM!

@arminmeh
Copy link
Contributor

arminmeh commented Aug 6, 2025

Every grid demo (also on master) throws a console error:

MUI X: useResizeContainer - The parent DOM element of the Data Grid has an empty height.
Please make sure that this element has an intrinsic height.
The grid displays with a height of 0px.

Something to be fixed as a part of this PR?

@romgrk
Copy link
Contributor Author

romgrk commented Aug 6, 2025

I prefer not to mix issues, this is just for the refactor, I'm fixing the new bugs as separate PRs. It makes it easier to review things.

@romgrk romgrk merged commit b8b0bd3 into mui:master Aug 11, 2025
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal Behind-the-scenes enhancement. Formerly called “core”. type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants