You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The core idea around why this approach can help bring value:
Put constraints in place to maximize the consitency of the developer experience on the whole library (user side).
Make it easier for an engineer to contribute to the library, as a whole, no matter which component we are used to working on. Experimentations should go through small efforts / RFC and be rollout at 100% when successful, not scattered between components.
Share knowledge, tools, and great practices learned the hard way. Any small incremental improvement to the CI, tests, etc compound. It can be very hard to share and enforce different repositories.
List of items
Export default right from the function declaration to increase code density. Meaning, no export default BasicDatePicker;.
Always spread the props on the root element if public. always. When not possible, consider breaking the component down into smaller chunks or introducing a xxxProps prop. I have seen the following issues. For instance:
Migrate from enzyme shallow (eww) to enzyme render (good) to testing-library (great). This is a work in progress on our side with the core components. We would upgrade the test anytime we find the opportunity, for instance when fixing a bug.
Have all the components using describeConfirmance.js. This help ensures the API of the component match the common conventions.
Replace the usage of mergeRefs with useForkRef (in core/utils). This hook avoided the ref cleanup cycle at each render.
Replace the usage of createDelegatedEventHandler with createChainedFunction (in core/utils). This avoids duplication in the bundle for the same feature.
@dmtrKovalenko Definitely :). Should we have a call? Otherwise happy to continue the conversation here in the writing (best so we can come back at it in the future) 👌
The purpose of this issue is to identify misalignment between the main repository and this one, see what improvement we can bring in any direction.
This issue fits into this effort mui/material-ui#19706.
The core idea around why this approach can help bring value:
List of items
export default BasicDatePicker;
.import React, { useState } from 'react';
withimport React as * from 'react'; React.useState
. [core] Migrate to import * as React from 'react' material-ui#19802.displayName
for component. React dev tools can infer the display name from the name of the function, plus the display names are kept in production, increasing the bundle size and reducing obfuscation. For instance: https://github.com/mui-org/material-ui-pickers/blob/502118dbc268f77ca510bb78d6d588ea96ec4161/lib/src/DateRangePicker/DateRangePickerDay.tsx#L161React.createContext()
API. The motivation is explained in [core] Add displayName to contexts material-ui#18468. For instance: https://github.com/mui-org/material-ui-pickers/blob/502118dbc268f77ca510bb78d6d588ea96ec4161/lib/src/LocalizationProvider.tsx#L6xxxProps
prop. I have seen the following issues. For instance:<A children={<B />} />
with the idiomatic React way:<A><B /></A>
. For instance: https://github.com/mui-org/material-ui-pickers/blob/502118dbc268f77ca510bb78d6d588ea96ec4161/lib/src/DatePicker/DatePickerToolbar.tsx#L65-L67root
. For instance: https://github.com/mui-org/material-ui-pickers/blob/502118dbc268f77ca510bb78d6d588ea96ec4161/lib/src/DateRangePicker/DateRangePickerDay.tsx#L124https://github.com/mui-org/material-ui-pickers/blob/502118dbc268f77ca510bb78d6d588ea96ec4161/lib/src/views/Calendar/Month.tsx#L62
mergeRefs
withuseForkRef
(in core/utils). This hook avoided the ref cleanup cycle at each render.createDelegatedEventHandler
withcreateChainedFunction
(in core/utils). This avoids duplication in the bundle for the same feature..defaultProps
. See [core] Remove all .defaultProps usages material-ui#16542 for reference. For instance https://github.com/mui-org/material-ui-pickers/blob/7bed283699ef768936a3ec7c5dc89571492dddd4/lib/src/views/Calendar/Day.tsx#L232Already with standalone issues
makeStyles
withwithStyles
.If anything is unclear or you would like to start a discussion on the why. Let me know :).
The text was updated successfully, but these errors were encountered: