-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[DataGridPro] Make @mui/x-data-grid-pro import shared code from @mui/x-data-grid
#3688
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
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.
| { "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/srcImport the shared code from
@mui/x-data-gridinstead 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/srcand droppackages/grid/_modules_/Migrate
@mui/x-data-grid,@mui/x-data-grid-proand@mui/x-data-grid-generatorto the core bundling strategy (see [core] Migrate@mui/x-license-proto the new bundling strategy #3738)In
@mui/x-data-grid-pro, replace the imports from the the root of@mui/x-data-gridprefixed withunstablewith imports from@mui/x-data-grid/internals/xxxStop 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