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] Agnostic styling #16677

Draft
wants to merge 80 commits into
base: master
Choose a base branch
from
Draft

Conversation

romgrk
Copy link
Contributor

@romgrk romgrk commented Feb 21, 2025

Part of the design-system agnostic work.

New styling solution.

Summary

This PR implements the datagrid styling solution as a new babel plugin that transpiles css() calls to static CSS classnames. The styles are extracted to a separate stylesheet for production mode. In dev mode, the css() calls are injected at runtime. @mui/x-internals/css contains shared logic that is used by the production and development codepaths.

To ensure that classes ordering stays consistent across prod & dev modes, the babel plugin builds an import graph to determine in which order the dev mode css() calls would be evaluated (and we can assume those are always at init time, because we only support static styles). In practice, this means styles defined in leaf imports will be defined after styles in non-leaf imports.

We are abusing a bit the babel build process, as babel plugins are meant to transform one file at a time, not to create new files. We attach a nodejs beforeExit handler to write our final CSS stylesheet. It's not very clean, but it avoids the need to mess further with the build process.

Code transformation examples

For an overview of what is supported, see the test cases: index.test.js

We only support static styles, except for vars.x.y.z, for which we do a bit of hardcoding/magic in order to make it work (the alternative would be manually writing var(--DataGrid-t-x-y-z).

Changes

I've only included the implementation of the babel plugin here, except for GridColumnHeaderTitle which I've used as an example of what would change for the rest of the codebase. I could complete the styling changes as a separate PR to facilitate the review, or as part of this one to ensure everything works before merging.

@romgrk romgrk added the component: data grid This is the name of the generic UI component, not the React module! label Feb 21, 2025
@mui-bot
Copy link

mui-bot commented Feb 23, 2025

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

Generated by 🚫 dangerJS against 43d7008

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

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

@romgrk
Copy link
Contributor Author

romgrk commented Mar 21, 2025

The workflow I proposed should allow to build production docs using the aliases

That's not a good thing though, the aliases mean we build the docs (which is basically the only place where most of us dogfood our products) through a substantially different setup than what end-users use. Now that I've been trying to get rid of the aliases, I was able to notice a few ESM issues that we might not have caught before release otherwise. I also remember some perf benchmarks giving different results when built inside our docs (in a playground page) versus in a newly setup project using the NPM modules.

The aliases are fine for dev mode to get HMR, but prod mode should not use them.

A babel plugin that reaches outside of its abstraction level. This is debt because it breaks the natural assumptions we have about our babel setup and thus facilitates introduction of regressions.

I don't like it either but it's going to stay as it is for now considering our timeline. Using something other than babel would mean introducing a new build tool in our build pipeline, and I don't want to investigate that.

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 Mar 23, 2025
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.

7 participants