-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[DataGridPro] Make @mui/x-data-grid-pro
import shared code from @mui/x-data-grid
#3688
[DataGridPro] Make @mui/x-data-grid-pro
import shared code from @mui/x-data-grid
#3688
Conversation
… GridApi and GridApiRef
These are the results for the performance tests:
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll do a more detailed review once #3953 is ready. With the _modules_
folder it's complicated to see the impact of this PR.
@@ -115,18 +119,17 @@ | |||
{ "name": "GridColumnMenuState", "kind": "Interface" }, | |||
{ "name": "GridColumnOrderChangeParams", "kind": "Interface" }, | |||
{ "name": "GridColumnPinningApi", "kind": "Interface" }, | |||
{ "name": "GridColumnPinningMenuItems", "kind": "ExportSpecifier" }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It needs to be exported to avoid a breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, I'll do a double check on the export files
import { GridColDef } from '../../../models/colDef/gridColDef'; | ||
import { GridPinnedPosition } from '../../../models/api/gridColumnPinningApi'; | ||
import { GridApiPro } from '../../../models/api/gridApiPro'; | ||
import { GridColDef } from '@mui/x-data-grid'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we would have a Pro GridColDef
, instead of using module augmentation.
As example:
import { GridColDef } from '@mui/x-data-grid'; | |
import { GridColDef } from '../../../gridColDef'; |
But we can see how this approach works in the long term.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should have a pro GridColDef
at one point
But it was creating too many problems with the api
params deep inside some GridColDef
callbacks.
So I went for the easy way for now.
@m4theushw for you deep review, I propose to do it on the new bundling strategy PR (after #3953) |
Part of #924
Fix #2245
Codesandbox playground for DataGridPro
Note: The proptypes generation is broken for pro-only components. Will be fixed in #3953.
In this PR
Move almost all the pro-only code into
packages/grid/x-data-grid-pro/src
Import the shared code from
@mui/x-data-grid
instead ofpackages/grid/_modules_
This code should not be released as is, we should drop the whole
_modules_
folder before to avoid problems.What's next ?
Move all the community code into
packages/grid/x-data-grid/src
and droppackages/grid/_modules_/
Migrate
@mui/x-data-grid
,@mui/x-data-grid-pro
and@mui/x-data-grid-generator
to the core bundling strategy (see [core] Migrate@mui/x-license-pro
to the new bundling strategy #3738)In
@mui/x-data-grid-pro
, replace the imports from the the root of@mui/x-data-grid
prefixed withunstable
with imports from@mui/x-data-grid/internals/xxx
Stop listing the pro-only events in
GridEvents
(we will probably need to replace the enum withtype GridEvents = keyof GridEventLookup
, which is a breaking change) - Need v6Stop listing the pro-only components in
GridSlotsComponent