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

[Data Grid] Add experimental headless exports #15662

Closed
wants to merge 3 commits into from

Conversation

lauri865
Copy link
Contributor

@lauri865 lauri865 commented Nov 28, 2024

Trying to address my main gripe with the datagrid – we love the core of it, but we don't use Material UI. We have all custom slots in place already, but we cannot get rid of the default slots from our bundle. This is one suggested way to do this for now.

Adds a possibility to import:

import {
  Unstable_DatagridPro as DataGridPro,
} from '@mui/x-data-grid-pro/headless';

This would prompt the developer to provide all of their own base slots and icons (excl. core components like rows, etc.).

Addresses #10143, #12370, etc. partially at least, for the time being.

An alternative option would be to put all the material slots into a separate package, so we could, within our build tooling, resolve it easily to an internal file that exports an empty object.

Very open to feedback, if you think there's a better way to achieve this.

@mui-bot
Copy link

mui-bot commented Nov 28, 2024

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

Generated by 🚫 dangerJS against a2ad678

@lauri865 lauri865 force-pushed the experimental-headless-exports branch from 266c113 to 9904e6a Compare November 28, 2024 19:37
Copy link

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

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 28, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 28, 2024
@oliviertassinari oliviertassinari added the component: data grid This is the name of the generic UI component, not the React module! label Nov 28, 2024
@flaviendelangle
Copy link
Member

The team is currently working to split the DataGrid logique and the @mui/material / @mui/emotion dependency.
The goal is not only to remove the components from the bundle size, but also the whole styling stack.

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

github-actions bot commented Dec 2, 2024

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

@romgrk
Copy link
Contributor

romgrk commented Dec 2, 2024

Thanks but I'm already working on this, there's many aspects to take into consideration and this approach is too limited for what we're trying to do. I should have something available on the v8 alpha in a month or two. I'll close this PR to avoid duplicating work.

@romgrk romgrk closed this Dec 2, 2024
@lauri865
Copy link
Contributor Author

lauri865 commented Dec 2, 2024

Makes sense. Would you accept a simpler PR though where I move all material slot imports into a single file per datagrid type (community, pro, premium)? Currently, the import resolution tree is so complex, that we'd have to patch all 3 packages to remove material imports in the upper layer.

Currently:
DATA_GRID_PREMIUM_DEFAULT_SLOTS_COMPONENTS imports its material components, but also imports DATA_GRID_PRO_DEFAULT_SLOTS_COMPONENTS which imports material components and also imports DATA_GRID_DEFAULT_SLOTS_COMPONENTS which imports material components.

It'd be a pretty simple change to have material "themes" resolve their own components, which would then make it easier to rip them out as well.

While I'm sure you can come up with a 10x approach, I'm also quite certain it will have to come at a steep cost for adoption / migration. Which will take time, and thus a stopgap solution would be greatly appreciated.

@lauri865
Copy link
Contributor Author

lauri865 commented Dec 2, 2024

Something along these lines:
https://github.com/mui/mui-x/blob/82d46ab329a11bf9308e8a1cf57786ac0cec37ad/packages/x-data-grid-premium/src/material/index.ts

import type { GridPremiumIconSlotsComponent } from '../models';
import {
  GridWorkspacesIcon,
  GridGroupWorkIcon,
  GridFunctionsIcon,
  GridSendPromptIcon,
  GridRecordPromptIcon,
} from './icons';
import materialSlotsCommunity from '@mui/x-data-grid/material';
import materialSlotsPro from '@mui/x-data-grid-pro/material';

const iconsSlots: GridPremiumIconSlotsComponent = {
  columnMenuUngroupIcon: GridWorkspacesIcon,
  columnMenuGroupIcon: GridGroupWorkIcon,
  columnMenuAggregationIcon: GridFunctionsIcon,
  toolbarPromptSendIcon: GridSendPromptIcon,
  toolbarPromptRecordIcon: GridRecordPromptIcon,
};

const materialSlots = {
  ...materialSlotsCommunity,
  ...materialSlotsPro,
  ...iconsSlots,
};

export default materialSlots;

@romgrk
Copy link
Contributor

romgrk commented Dec 2, 2024

I'm planning to do a refactor of that kind in the next weeks, and I think I already have it done to some extent in #12432 (though I'll probably reuse that PR for parts).

I will however be working on getting rid of material-ui style-engine and typings first, so this part (the components) would be the last one of the design-agnostic refactor.

Would you accept a simpler PR though where I move all material slot imports into a single file per datagrid type (community, pro, premium)?

I understand that this is a priority for you so if you want to make a very very simple PR that's enough to keep you happy for the time being, I will review it, but I can't guarantee that we will merge it if there is a potential conflict with the ongoing work. In other words, I don't recommend that you do it, but it's not a strict no.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module! PR: out-of-date The pull request has merge conflicts and can't be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants