Coder Social home page Coder Social logo

mui / mui-x Goto Github PK

View Code? Open in Web Editor NEW
3.2K 53.0 854.0 111.36 MB

MUI X: Build data-rich applications using a growing list of advanced React components.

Home Page:

JavaScript 1.67% TypeScript 98.30% HTML 0.02%
react data-grid data-table date-picker date-range-picker time-picker hacktoberfest charts

mui-x's People


alexfauquette avatar bosselijah avatar cherniavskii avatar clytras avatar coldatnight avatar danailh avatar dependabot-preview[bot] avatar dependabot[bot] avatar dtassone avatar flaviendelangle avatar giladappsforce avatar gitstart avatar janpot avatar joserodolfofreitas avatar lindapaiste avatar lukasty avatar m4theushw avatar mbilalshafi avatar michallukowski avatar michelengelen avatar mnajdova avatar noraleonte avatar oliviertassinari avatar renovate[bot] avatar romgrk avatar samuelsycamore avatar sebastianfrey avatar siriwatknp avatar yaredtsy avatar zeeshantamboli avatar


 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar


 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

mui-x's Issues

[DataGrid] Implement i18n

[core] Bundling vs dependencies

In its current form, the released packages bundle code instead of relying on a nested dependency tree.

For instance:

  • The @material-ui/x package bundles @material-ui/x-license & @material-ui/x-grid & @material-ui/x-tree-view
  • The @material-ui/x-grid package bundles @material-ui/x-grid & @material-ui/x-license
  • The @material-ui/x-tree-view package bundles @material-ui/x-tree-view & @material-ui/x-license

I can't help myself asking, wouldn't it be better to leverage transitive dependencies? In this mode, we would get this tree structure:

    ├── @material-ui/x-grid
    │   └── @material-ui/x-license
    └── @material-ui/x-tree-view
        └── @material-ui/x-license

Which can avoid duplicates in developer's bundle when they start to do fancy stuff, like importing from @material-ui/x and @material-ui/x-tree-view at the same time.

Continuous Integration

A list of checks we will need to run in the CI in order to save us time during development and help guarantee a high code quality:

A lot should be copied from, the CI has been gone through numerous iterations there already. It will be much better to compound on it. Then, once we double the size of the team, I think that it could starts to make sense to diverge, exploring different directions.

Early feedback

This issue was originally opened in, 10/04/2020, work was transferred to this repository, for now.

Notes I have taken while looking at the source. Some are more relevant than others. I have been as exhaustive as I could.

  1. I think that we can gain efficiency by replacing with
import * as React from 'react';

It's the approach we use in the main repository. It avoids back and forth with the top of the file when refactoring.

  1. If we could use the same prettier config as the mono-repository, it would help to switch between projects. from to:
module.exports = {
  printWidth: 100,
  singleQuote: true,
  trailingComma: 'all',
  overrides: [
      files: '*.d.ts',
      options: {
        // This is needed for TypeScript 3.2 support
        trailingComma: 'es5',

I was reading the code and noticed that most of the lines were going off the screen. 180 chars per line is a lot 😬.

  1. I see that you have started to design an initial API. While not the priority, I would love to get your feedback on the API I started to implement in the past (from #18872).
  2. Regarding the size of the files. I'm looking at the models as an example. I'm more in the camp of having 1,000 LOCs per file, I don't mind, quite the opposite, I love how easy it makes refactoring and searching. My only point is that if you break the models into different files so I could more easily navigate the source, it's not needed.
  3. forwardRef to add at some point ;)
  4. You can assume px is the default unit:
-<DataContainer ref={gridRef} style={{ minHeight: renderCtx?.totalHeight + 'px', minWidth: internalColumns.meta.totalWidth + 'px' }}>
+<DataContainer ref={gridRef} style={{ minHeight: renderCtx?.totalHeight, minWidth: internalColumns.meta.totalWidth }}>
  1. Love the style-wrapper approach. I was thinking of making a components in v5 to enable developers to inject custom versions. I think that it will be interesting to look at the API of, it's a prior-work to solve the theming problem.
  2. I doubt we need to force a display name on the elements, for instance: What's the objective?
  3. A note on arrow function vs named function. We try to follow this approach: named function for top-level scope, arrow function for nested scopes. Could it work for you?
  4. Jira account created. I'm in favor of a methodology where we make bets on specific topics with a fixed deadline. So more scrum than kanban.

Capture d’écran 2020-04-09 à 23 55 53

I like the idea behind

  1. There is a bunch of effect dependency warnings. I don't think that we should ignore any. I'm happy to help with any of them.
  2. In the onScroll listener. Would it be worth looking at pruning the logic for the x or y-axis if it wasn't scrolled?
  3. We might have a 1px offset computation issue. Scroll at the bottom, nothing happens in the first 10% of the scrollbar range.
  4. Does the grid support IE 11? Should we ignore it? (I wonder with position sticky)
  5. If you are only using debounce from lodash, you could import it from import { debounce } from '@material-ui/core/utils'; It's a 10 LOCs method. Speaking of debounce, we should clear it when you unmount the component, otherwise, we risk crashing the app.

Great start 👍

[XGrid] Column resizing broken in StrictMode

I opened up a new react project to test x-grid and it works for the most part, but resizing doesn't seem to work at the moment.

I've set up my project like this:

import React from 'react';

import { XGrid, RowsProp, ColDef, LicenseInfo } from '@material-ui/x-grid';


const rows: RowsProp = [
  { id: 1, col1: 'Hello', col2: 'World' },
  { id: 2, col1: 'XGrid', col2: 'Awesome' },
  { id: 3, col1: 'Material-UI', col2: 'Amazing' },

const columns: ColDef[] = [
  { field: 'id', hide: true},
  { field: 'col1', headerName: 'Column 1', resizable: true },
  { field: 'col2', headerName: 'Column 2', resizable: true },

function App() {
  return (
    <div className='App' style={{ height: 400, width: '100%', display: 'flex', flexGrow: 1 }}>
      <XGrid columns={columns} rows={rows} />

export default App;

The drag button does seem to work but nothing happens. I also get this error inside the console when i try to drag the columns:

index-esm.js:15 Uncaught TypeError: Cannot read property 'field' of undefined
    at fr (index-esm.js:15)
    at index-esm.js:264
    at HTMLDivElement.<anonymous> (index-esm.js:264)

[DataGrid] Implement Column reorder

[RFC] Use Babel

In order to build on top of the same foundation and to save time, I think that we should reuse the existing build steps of the main repository by leveraging Babel. It appears that reproducing the same building pipeline will provide the maximum consistency. For instance

Running "npm run start" fails to compile

I pulled the latest changes and after running the project locally it fails to compile. Running the build also doesn't work.

The issue I'm getting is related to missing next folder inside the docs folder.

The commit from which this problem started is this one -> #144

PS: If this is not the right way to create issues please point it out and I'll update it.

[DataGrid] Prefix exported types and modules

I think that we should make sure that all the exported types and modules are unique within the Material-UI ecosystem, this includes all the packages in @material-ui/*. We had a first discussion around the issue on

This is important for two reasons:

  • Avoid all name conflict when importing, as we will re-export everything from @material-ui/x.
  • Avoid all confusion when searching on Google, in-site Algolia, or even inside your own codebase.

We can benchmark with competitors:

I don't think that Kendo is the leader by chance, they have put a lot more care into the design of their components. I think that Grid would make a great prefix. In, we already do our best to avoid conflicts

[RFC] Change API state

Looking at the current shape of the API, it seems that we have an opportunity to explore a more intuitive and flexible API around how the different states can be handled. We have started to uncover this aspect in #133, where we talk about renaming sortModel to sortBy. We have also uncovered some of the problems in #165 (comment) with the update columns method.

Changes I would propose:

  1. It doesn't make sense to have a controllable prop and an imperative method that can update a state at the same time. It's confusing, it can lead to nasty bugs when the grid rerenders with the controllable prop and undo the imperative calls. If we take the sortModel state, I think that setSortModel should warn when used without the uncontrollabe API: defaultSortModel.
  2. The corollary is that we should introduce a defaultSortModel prop to allow the uncontrollable API to function with the imperative API setSortModel.
  3. Update of the defaultSortModel prop could be supported, and equivalent to calling setSortModel.

This gives us:

  • sortModel: the controllable prop, will be used no matter what as a source of truth.
  • defaultSortModel: the uncontrollable prop, the state is hosted by the component
  • setSortModel: imperative API to update the uncontrollable mode, update the state hosted by the component. Throw or warn if the state is already controlled.
  • onSortModelChange: callback method used for the controllable mode.
  • getSortModel: imperative API to pull the state hosted by the component.

Another corollary is that we should remove duplicated APIs, for instance: should be removed for simplicity and clarity.

In the documentation, I think that we should:

  • rely on the uncontrollable mode when possible
  • have the controllable mode documented before the imperative API

While in this demonstration, I take the "sortModel" state as an example, I think that we can apply the very same model to all the other states. In the core package, there is a useControlled() hook to normalize the behavior. Example of other states we can expose:

This should solve my concerns from #165 (comment). Exposing a resetState publicly doesn't feel right, not at all. From what I understand the boolean influences: caching and internal states. Caching should stay private and internal states should be exposed independently (columnsWidth).

Discrepancy with main repository

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.

While it's unclear how this package will evolve in the future, staying standalone or merging with the main repository as we plan for the pickers. So far the great reason not to merge is to keep the effort private and allow fast POC iterations. But what about once we make it public? I think that our default answer to the problem would be to go all-in in the mono-repository approach unless there is a great reason not to, something that would outweigh the cost (like we have right now).

The core idea around why this approach can help bring value:

  1. Put constraints in place to maximize the consitency of the developer experience on the whole library (user side).
  2. 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.
  3. 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

Left is mui-org/material-ui-x, right is mui-org/material-ui. I will make the case for why the right approach might be better at the same time. I would love to hear counter-arguments in the other direction.

[RFC] Remove dependency on @material-ui/icons

The line I'm going after:

While there are a couple of leverages we can use to make a hard dependency on @material-ui/icons more viable, I don't think that it's a wise direction to take. I would propose we apply the same tradeoff than with all the other packages. We inline the few SVG icons we need. It's a small bundle size duplication overhead. To the best of my knowledge, we wouldn't have a lot of them. Here are examples:

Color Accessibility threshold

Potential issue

Capture d’écran 2020-05-05 à 18 29 56

I see two issues in the above case:

  1. The color contrast of the checkbox is 1.39:1 It fails to match the WCAG AA minimum accessibility target (4.5:1).
  2. Luckily, Google has us covered 🙂

Potential solution

I think that we can solve the above problems with:

  1. Use the primary color for the Checkbox
  2. Patch the selected background color until we update the value to match the specification in the core. 8% of the opacity of the primary in light mode and 16% in dark mode.

[DataGrid] Implement Column pinning

[DataGrid] Implement CSV export

[data grid] Implement Column groups

Re-evaluate requirement for tslib as a dependency

This is a follow-up on a conversation we have started in #44 (comment). The tslib dependency was added in order to make codesandbox runs successfully. It's unclear if the requirement is coming from codesandbox itself or from a misconfiguration of the building pipeline.

It's also to be noted that we use babel-runtime on @material-ui/core which might duplicate in the bundle for our users.

@dmtrKovalenko Considering we have a similar building stack on the pickers, do you have any idea of what could be wrong? :)

I have added the "Post MVP" milestone as we can/should delay the resolution to later.

Improve CSS architecture

3 improvements opportunities:

  1. Make specificity as low as possible. I have discovered this looking at the component, there are cases where we will need to allow the developer to customize the rendering of the component, we should identify these cases and make sure the CSS specificity is as low as possible. There are specificity over 5 (barely overridable) in:
    The higher the specificity is the harder the overrides are.
  2. Differentiate class names that are public and private. It's unclear, looking with the Chrome dev tool, which classes are private, and which ones are public to be customized.
    For the name of the class names, let's use the same convention as the main repository.
  3. Reduce bloat. I think that we should drop styled-components for the first MVP to simplify the usage of the grid (one less dependency to install & to configure for SSR & injection order), leverage the values from the theme (easier overrides), reduce the bundle size (-12 kB gzipped).

[DataGrid] Remove dependency on lodash

While lodash was inlined in the repository (I guess to hide, on the surface, the dependency on this library), I don't think that it removes the fact that we depend on it. I would be in favor of completely removing its modules from the codebase.

Possible migration plan:

  • denounce. Instead of the 938 B version of lodash we could rely on the 305 B import { denounce } from @material-ui/core module. We could also move the module to @material-ui/utils instead of the core if that makes more sense. This module also has the advantage of being deduplicated with the core components, sharing the simple source.
  • deep equal. Instead of the 3.8 Kb version of lodash, we could replace it with fast-deep-equal, a 414 B alternative. I have only seen one use case in the codebase, I even wonder if we couldn't use JSON.stringify directly.

[RFC] Increase the scope of React peer dependencies down to 16.8.0

For consistency with the core packages and compatibility with the ecosystem, I think that we can widen the scope of the support versions of React from ^16.13.1 to ^16.8.0.

I'm not aware of any downsides to doing such, only upside with consistency against the other Material-UI's packages. The day we get a bug from somebody that uses an older version and we can fix by increasing the minimum version of React supported, sure.

priority: small, cost: small

[RFC] Should we host MUI X on the main repository?

The idea to host the X line product in the main repository is growing in me. While we are going in this direction for the pickers: mui/material-ui#19706, I would like to continue the reflection for X. It's not obvious.


  • Better integration and coherence between the MIT and paid content: more code shared, more knowledge shared.
  • Avoid doing back and forth between the source and the documentation. From what I understand, the workflow will be as follow: make the change in this repository, wait for the next release schedule, (1-week frequency?) once the code is released, go to the main repository, update the documentation, wait for the next release schedule (1-week frequency). Live.
  • Share the same tooling, I have started to look at the tooling gap. I believe #23 and #24 are 5% of what's left to address. CircleCI, codesandbox-ci, bundle size tracking, test coverage, visual regression, Netlify deployment, cross-browser end to end tests (need to replace Jest with Mocha).
  • Else?


  • Less audience isolation. We risk paid content perverting and negatively influencing the MIT community.
  • Slower CI, which is the price for better integration. We can mitigate it by spending $ on better machines ($0/month so far) and introducing parallelism.
  • Else?

Can it scale?

Looking at larger projects, yes, it seems so

Note that if bug tracking is a concern, we can very well manage the concern independently to where the source is hosted: fullcalendar/fullcalendar-workspace#558.


cc @mui-org/core

setState leak after unmount

@dtassone The issue was caught in the CI (Karma), something is calling setState after the component is unmounted. It leaks.

Capture d’écran 2020-08-05 à 00 32 02

@eps1lon this makes me think that we might have an improvement opportunity in the CI, shouldn't we fail if a component call console.warn or console.error during the tests? I believe it works when we run the tests in JSDOM, but not with Karma. I don't know why yet. The data grid might be an exception as all JSDOM tests are skipped (require layout).

[DataGrid] Bundle size is way too large

The current bundle size and bundling strategy is not flying. It's not living up to the quality level our audience expects from the core components.


First, tree-shaking is not working. (tested with CRA). This means that, for instance, developers bundle all the locales. I think that the solution is to drop the current bundling strategy and copy what the core is doing. I think that this bundling problem is hard enough, we don't have the resources to solve the problems twice because we use two different solutions between different packages.


import * as React from 'react';
import { GridToolbar } from '@material-ui/data-grid';

export default function DataGridDemo() {
  console.log('GridToolbar', GridToolbar)
  return null;

out, in the production bundle

Screenshot 2021-04-24 at 20 16 03

Capture d’écran 2021-06-12 à 12 44 35


Second, here is a couple of bundle size-reduction opportunities:

  • clsx. (win ~0.3 kB gzipped) Replace classnames with clsx.
  • license. (win ~0.7 kB) I believe we can remove the need from including packages/license/src/encoding/base64.ts in the bundle of the users. The possible solution would work as follow:
    • client-side, use the built-in window.atob and window.btoa, only run the license check if the window is defined (ignore server-side, and CI)
    • server-side, us the built-in Buffer.from('Hello World!').toString('base64'); and Buffer.from(b64Encoded, 'base64').toString() utils.
  • lodash. The dependency was inlined in the repository (I guess to hide, on the surface, the dependency on this library), I don't think that it removes the fact that we depend on it. I would be in favor of completely removing its modules from the codebase.
    Recommended migration plan:
  • denounce. (win ~ (0.5 kB) Instead of the 938 B version of lodash we could rely on the 305 B import { denounce } from @material-ui/core module. We could also move the module to @material-ui/utils instead of the core if that makes more sense. This module also has the advantage of being deduplicated with the core components, sharing the simple source.
  • deep equal. (win ~3.5 kB gzipped) Instead of the 3.8 Kb version of lodash, we could replace it with fast-deep-equal, a 414 B alternative. I have only seen one use case in the codebase, I even wonder if we couldn't use JSON.stringify directly (see #3954)
  • Select. (win ~0.5 kB gzipped) Replace Select import with NativeSelect).
  • TextField (win ~5 kB gzipped). Replace TextField with individual imports, allow dropping OutlinedInput and FilledInput from the bundle..
  • Babel (win ~0.6 kB gzipped) Remove propTypes in production with this Babel plugin.

[XGrid] Column resizing is too limiting

I believe that extending the resizing session up to the mouse up event would improve the UX. I don't think that we should interrupt the resizing session when the mouse leaves the column header. This surface area is relatively small. It would be better for end-users not to have to concentrate on keeping the same vertical alignment with their pointing device. It also feels weird to have the mouse pressed but not doing anything (once escaping the zone).

Aug-19-2020 17-36-35

Aug-19-2020 17-39-46

Cells typography

Potential issue

I'm going to assume that we want to build a first theme for the preview of the DataGrid. In this context, we want to follow the Material Design specification: I have seen the following divergences with the specification: the typography:

Check the implementation of TableCell for a version that is supposed to (unless proven otherwise 🙃) matches the specification.

Potential solution

The solution would be about:

  • using theme.typography.body2
  • using theme.palette.text.primary
  • loading the Roboto font instead of using the system font

Actually, I wonder. Would the performance be good enough if we used the component? (I suspect it won't be but it would be interesting to figure that out. Alternatively, would it make sense to only import the style of the component?

[core] Improve packages folder?

@dtassone What about we:

  • Rename packages/license folder -> packages/x-license-pro to match package name
  • Flatten the packages/grid folder to make it easier to navigate the codebase?

Remove React.memo() anytime a react element is accepted

While the default strategy should be to not use React.memo() unless there is a good reason to use it cc @eps1lon, looking at the codebase, there is a quick win on the problem. In 99% of the cases, anytime a React element is accepted as a prop, React.memo() is a waste of CPU cycles (unless this babel plugin kicks in, which is not frequent).

Components we can make faster be removing memo:

[RFC] Package structure

This is a follow-up on a couple of past discussions we had, mostly speaking. I couldn't find any written conclusion on how to best name the package and components outside of a verbal discussion that no other member can introspect. In order to improve decision making and transparency, I'm making a written RFC, so we can settle.


We came to the conclusion at that:

  • All the components exposed in Material-UI are in a single namespace.
  • How we structure the npm packages should be only driven by "deliver" concerns. So far:
    • isolating MIT and enterprise licenses, legal concerns: @material-ui/core vs @material-ui/x.
    • testing if it helps increase market share, also help to provide KPI to assert the quality of our work: @material-ui/pickers.
    • share code between multiple packages: @materal-ui/utils.


Looking at the current naming organization, I see a couple of things we could potentially improve:

  • The Grid component name is already reserved by We can't use it here.
  • I believe it would be better not to have an intermediary /grid folder. A flat structure would be easier to navigate.
  • It seems that the intermediary /grid was introduced to fix a naming organization issue: if we sort ASC, related components won't be grouped. This might be an issue we would want to solve userland too (not only in the repository).


A couple of options that we could consider to solve the above:

  1. We could put emphasis on the components, paid or not. This would enable to flatten the file structure. Moving from:



I've not convinced it's better, but it could have its advantages, to consider.

  1. We could simply remove the intermediary /grid folder. I think that it would make it easier to navigate the codebase.
  2. We could consider that the Enterprise version of the component is meant to completely replace and extend the MIT version. I think that it would create a good constraint for the developer experience. We would only document one component, with props that are restricted to X with a dedicated notice.
    In practice, it would mean:
import { DatePicker } from '@material-ui/pickers';
import { DateRangePicker } from '@material-ui/x-pickers';
// import { DataGrid } from '@material-ui/data-grid';
import { DataGrid } from '@material-ui/x-data-grid';
  1. In the even 3. it not identified as the best approach, we can differentiate the two components (free and paid):
import { DatePicker } from '@material-ui/pickers';
import { DateRangePicker } from '@material-ui/x-pickers';
import { DataGrid } from '@material-ui/data-grid';
import { XGrid } from '@material-ui/x-grid';

[DataGrid] Implement multi-column filter

Plan of action

  • 1. Build a column header menu, it will be used for filtering, sorting and any other feature that a column can leverage.

Capture d’écran 2020-11-02 à 11 45 48

  • 2. Build the internal state filtering logic to support one type: string and one operator: contain.
  • 3. Create a global filter UI popup, same as Airtable, Notion, Retool.

Capture d’écran 2020-11-02 à 11 46 14

  • 4. Expose a programmatic public filtering API (3. was for end-users, 4. is for developers).
  • 5. Support all the operations and types possible: date, date range, exclude, etc.
  • 6. Add a toolbar component #516 (filter popup open, display density, etc.)

Capture d’écran 2020-11-02 à 11 47 07

  • 7. Implement quick filter #202

Capture d’écran 2020-11-02 à 11 48 42

  • 8. Consider supporting inline filter (e.g. react-table demo) but we might not do it.

Capture d’écran 2020-11-02 à 11 47 44


Screenshots Capture d’écran 2020-10-01 à 00 11 07 Capture d’écran 2020-10-01 à 00 10 03 Capture d’écran 2020-10-01 à 00 08 11 Capture d’écran 2020-10-01 à 00 08 36 Capture d’écran 2020-10-01 à 00 06 56 Capture d’écran 2020-10-01 à 00 07 00 Capture d’écran 2020-10-01 à 00 05 20 Capture d’écran 2020-10-01 à 00 05 31 Capture d’écran 2020-10-01 à 00 06 03 Capture d’écran 2020-10-01 à 00 03 39 Capture d’écran 2020-10-01 à 00 01 35 Capture d’écran 2020-09-30 à 23 59 50 Capture d’écran 2020-09-30 à 23 58 38 Capture d’écran 2020-09-30 à 23 57 32 Capture d’écran 2020-09-25 à 22 53 14 Capture d’écran 2020-09-30 à 23 55 21

Bundle issue

Something is wrong with the integration of the grid with the main repository:

  1. The grid crashes on the main documentation if you try to render it

Capture d’écran 2020-07-23 à 12 36 07

  1. The grid crashes on the main testing suite if you try to render it:

Capture d’écran 2020-07-23 à 12 37 22

[DataGrid] Implement Cell editing

Exported types should live in a global namespace

I have noticed the following import when reading the documentation that made me wonder

Shouldn't we assume that all public API lives in a global namespace? Let's assume we have two components that have an Columns exported variable.

  • We plan to reexport all the components from @material-ui/x. I expect that most developers will use it for simplicity. Conflict
  • What about people that want to search? In their codebase, in Google, in the built-in search of the documentation?

I would recommend:

-import { Columns, RowsProp, XGrid } from '@material-ui/x-grid'; 
+import { GridColumns, GridRowsProp, XGrid } from '@material-ui/x'; 

Default boolean prop value should be false

Surface code review

Here are some of my foundings on the current material-ui-x codebase.

Do not take it personally, just some of my observations

  1. Not using clsx module to combine class names

Such expressions are so unreadable (

let cssClasses = `${CELL_CSS_CLASS} ${cssClass || ""} ${
  showRightBorder ? "with-border" : ""

cssClasses += !align || align === "left" ? "" : ` ${align}`;
  1. Classname expressions className={'country-flag'} are a little bit weird.
    Which is potentially caused by 1.
  2. The function component should have return all the time because any potential change that will require variable or state hook in the component will require to change the whole component code. Which harms git history.
export const IsDone: React.FC<{ value: boolean }> = React.memo(({ value }) =>
  value ? <DoneIcon fontSize={"small"} /> : <ClearIcon fontSize={"small"} />
export const IsDone: React.FC<{
  value: boolean;
}> = React.memo(({ value }) => {
  return value ? (
    <DoneIcon fontSize={"small"} />
  ) : (
    <ClearIcon fontSize={"small"} />
  1. Do we really need to wrap all the components in React.memo? Memoization costs runtime performance. And probably we do not need to make premature optimizations:

MOST OF THE TIME YOU SHOULD NOT BOTHER OPTIMIZING UNNECESSARY RERENDERS. React is VERY fast and there are so many things I can think of for you to do with your time that would be better than optimizing things like this. In fact, the need to optimize stuff with what I'm about to show you is so rare that I've literally never needed to do it in the 3 years I worked on PayPal products and the even longer time that I've been working with React.

  1. Avoid using inline styles style={{ display: 'flex', justifyContent: 'space-between' }} (, and other)

  2. Grid cell field prop must be well-typed string union which will autocomplete the other available props (

  3. Make sure that we are using strict mode in tsconfig.json. I am seeing some functions without argument types, that would be inferred as any

  4. Expressions like icons: { [key: string]: React.ReactElement }; are so unsafe. key must be well-typed for autocomplete and type system experience

  5. Probably we need some convention about the body of the functional component. IMHO it is much more readable when all the state/ref hooks are going first, then declaring effects, then callbacks and than return. It is similar to what we have with class components (1. state in the constructor, 2. methods, and the last render method) E.g. this component is really hard to read.

  6. Avoid using 1-letter variable names, even for maps {, idx) => ( (

  7. This .map loop is to long and too nested. Really hard to read, probably need to split in some reusable functions to improve readability.

  8. Do we really need constants for css classes? Looks like it is too different from material-ui styling solution (

  9. Events must be properly typed MouseEvent is native dome event. React.MouseEvent is react synthetic event and must be used as React.MouseEvent<HTMLElement> to prevent explicit casts later on and improve typings. (

Overall typescript doesn't work well right from what I am seeing. For example probably we want to use import * as React from 'react' in this project as well to get better access to the react typings.

Probably we don't need to use explicit type definitions everywhere like here:

  const getColumnFromField: (field: string) => ColDef = field => stateRef.current.lookup[field];
  const getAllColumns: () => Columns = () => stateRef.current.all;
  const getColumnsMeta: () => ColumnsMeta = () => stateRef.current.meta;
  const getVisibleColumns: () => Columns = () => stateRef.current.visible;

We could make the return value to be inferred automatically. The same for anonym objects.

It is used ! operator really rarely which means that we have an unsound type system and we are going to crash here 😱 just like here. Probably we want to use ? operator?

  1. Unnecessary spaces between imports. Used a lot, example from
import { useEffect } from 'react';

import { useLogger } from '../utils/useLogger';
import { KEYDOWN_EVENT, KEYUP_EVENT, MULTIPLE_KEY_PRESS_CHANGED } from '../../constants/eventsConstants';

import { GridApiRef } from '../../grid';
  1. Do we really need to use forceUpdate's in each hook? Probably we can avoid using it?

  2. Do we need to duplicate ColumnsContainer.displayName = 'ColumnsContainer'; for each component? It will be inferred by react automatically

  3. Is it intended that we are using global classnames? I suppose we need to remove usage of the global class names like here. It is potentially dangerous because of accidental overrides. But I suppose it will be redone.

  4. Probably we need to use prettier for code formatting, to increase readability for other contributors.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.