[core] Create a new package to share utils across X packages #13528
Conversation
| @@ -6,12 +6,12 @@ import { | |||
| } from '@mui/utils'; | |||
| import useLazyRef from '@mui/utils/useLazyRef'; | |||
| import useTimeout from '@mui/utils/useTimeout'; | |||
| import { useResizeObserver } from '@mui/x-utils/useResizeObserver'; | |||
There was a problem hiding this comment.
I need this hook in #13520 so I think it's a good candidate to bootstrap the package and make sure it works.
|
Deploy preview: https://deploy-preview-13528--material-ui-x.netlify.app/ |
| @@ -557,7 +557,6 @@ | |||
| { "name": "useGridSelector", "kind": "Variable" }, | |||
| { "name": "useGridVirtualization", "kind": "Function" }, | |||
| { "name": "useOnMount", "kind": "Function" }, | |||
| { "name": "useResizeObserver", "kind": "Function" }, | |||
There was a problem hiding this comment.
Was this export intented?
It was imported from @mui/x-data-grid/internals in the pro package so I guess it's was a mistake.
If it's intended, I can re-export it.
If it's not intended, I can re-export it and deprecate it so that we can remove it in the next major.
There was a problem hiding this comment.
I think it should be fine to remove it from the public exports 👍
There was a problem hiding this comment.
Thanks 🙏
I'll merge this PR once 7.8.0 is out 👍
flaviendelangle
force-pushed
the
x-utils
branch
6 times, most recently
from
77672ee
to
54b15e1
Compare
github-actions
bot
added
the
PR: out-of-date
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
github-actions
bot
removed
the
PR: out-of-date
There was a problem hiding this comment.
Wouldn't it be preferable to put the shared utils in the existing core utils package? This code looks like the kind of hooks we already have there.
There was a problem hiding this comment.
About the needs of an utility package that is specific to X:
We have been discussing the status of @mui/utils for years and I am reluctant to commit to much time on something that might change in the future (see this Notion page, we also have a Github issue that I failed to get my hand on.
For me the main goal here is to have an easy way to share code between the X components without entering the big discussions of "Should it be in @mui/utils? Should it be in @mui/base? Should it be in @mui/utils but X imports it from @mui/base? etc...
And to have it linked to our major cycle. If we start to add more complex utility functions (we were talking about drag & drop primitives or stuff around the data loading) I think it will become hard to have this code live in a package we do not own and with a major cycle we do not control.
There was a problem hiding this comment.
I agree with @flaviendelangle here. With time, each project will drift apart, and @mui/utils goals will be different from @mui/x-utils, given its fuzzy nature.
There was a problem hiding this comment.
With that being said, I think it's interesting to discuss what we want to have in a shared codebase at the company level and why.
I think that the model where everything that might be useful company wide is in the core is not a great approach because it makes us second-class users of those tools which comes with several problems:
- Major version synchronization
- Feeling of ownership (we are less likely to go improve a piece of code in the core repo to make it compatible with React 19 for instance)
- Complexity to add a utility function (we have to make sure the core releases it, we need to bump our version of
@mui/utils, whereas here we just need to release all the X packages like we always do) - Dependent of the core internal structure (the discussion around where those utils should live highlights this, I don't think we should depend on the core product structure to decide where to import a util like
useForkReforuseId)
But on the other hand, having some utils shared at the company level has the obvious benefit to avoid duplication which makes sure every package benefit from any improvement made to this utils.
My personal opinion is that, for topic that are related to the core like all the classes utilities, it make a lot of sense to consume them from a core package.
But for utils like the one listed in this PR (and even if I needed something like useForkRef and it did not exist), I don't think it's optimal.
|
During Friday's X general meeting, we decided to go for |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
github-actions
bot
added
the
PR: out-of-date
github-actions
bot
removed
the
PR: out-of-date
| @@ -557,7 +557,6 @@ | |||
| { "name": "useGridSelector", "kind": "Variable" }, | |||
| { "name": "useGridVirtualization", "kind": "Function" }, | |||
| { "name": "useOnMount", "kind": "Function" }, | |||
| { "name": "useResizeObserver", "kind": "Function" }, | |||
There was a problem hiding this comment.
I think it should be fine to remove it from the public exports 👍
Discussions
For you, what should be the name of this package?
@mui/x-utils?@mui/x-private-utilsto make clear that it's not intended for external usageDecision:
@mui/x-internalsOther utils to migrate in follow up
@mui/x-tree-viewand@mui/x-data-gridbuildWarningis used in pretty much all the X packagesgetActiveElementis used at least in@mui/x-date-pickersand@mui/x-data-grid