Skip to content
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

[SelectField][material-next] Introduce new component #21782

Open
pqr opened this issue Jul 13, 2020 · 6 comments
Open

[SelectField][material-next] Introduce new component #21782

pqr opened this issue Jul 13, 2020 · 6 comments
Labels
breaking change component: select This is the name of the generic UI component, not the React module! performance

Comments

@pqr
Copy link

pqr commented Jul 13, 2020

Minimum example of React Material-UI with the only TextField component imported - result bundle size is 250Kb!

https://github.com/pqr/react-mui-treeshake-does-not-work

webpack-bundle-analyzer shows everything is imported, nothing is tree shaked.

According to documentation https://material-ui.com/guides/minimizing-bundle-size/ tree-shaking should work, quote:

Tree-shaking of Material-UI works out of the box in modern frameworks. Material-UI exposes its full API on the top-level material-ui import. If you're using ES6 modules and a bundler that supports tree-shaking (webpack >= 2.x, parcel with a flag) you can safely use named imports and still get an optimised bundle size automatically:

import { Button, TextField } from '@material-ui/core';

But as shown in this minimum example it does not work out of the box.
I did several experiments, read through issues and StackOverflow and failed to find a solution.

  • [+] The issue is present in the latest release.
  • [+] I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

Bundle size of minimum app with the only TextField imported is about 250Kb, everything is included into bundle (analized with webpack-bundle-analyzer

Expected Behavior 🤔

Bundle should not contain any code not related to the only imported TextField, tree shake to be working

Steps to Reproduce 🕹

git clone git@github.com:pqr/react-mui-treeshake-does-not-work.git
cd react-mui-treeshake-does-not-work
npm install
npm run build
npm run analyze

Your Environment 🌎

Tech Version
Material-UI v4.11
React 16.13.1
Browser Any
TypeScript No
Webpack 4.43
@pqr pqr added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jul 13, 2020
@eps1lon
Copy link
Member

eps1lon commented Jul 13, 2020

That looks fine to me. It's more of an issue with how overloaded the TextField is. It currently includes the Select component which requires the Popover, Modal, List etc.

Tree-shaking is working or otherwise you'd see a lot more components. You can verify this by just importing the Button.

@eps1lon eps1lon closed this as completed Jul 13, 2020
@eps1lon eps1lon added discussion performance and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jul 13, 2020
@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 13, 2020

It's more of an issue with how overloaded the TextField is.

@eps1lon While we have been discussing (or even agreed) on moving the select use case outside of the TextField, with a new SelectField component. I'm not aware we have a dedicated issue for. What about we use this one for the purpose? It seems to match perfectly (the difference is that the author didn't realize the whole dependency chain of TextField that is quite large) If we do so, we can reopen until fixed :).

@oliviertassinari oliviertassinari added the component: text field This is the name of the generic UI component, not the React module! label Jul 13, 2020
@eps1lon
Copy link
Member

eps1lon commented Jul 13, 2020

This issue is about tree-shaking. Why is it so important to re-purpose that? What is that solving that a new issue can't?

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 13, 2020

@pqr wants his app to only include the code he needs. In his bundle size analysis

he noticed a lot of components with no clear relation to displaying an input. He went on assuming that it's because tree shaking is broken. Wrong.

So from what I understand, we are covering the same root pain point with the SelectField component proposal. It's only the interpretation of the origin of the pain from the author that is wrong, the underlying issue is still present: text field shouldn't bundle select.

The proposed solution is:

-<TextField select />
+<SelectField />

@oliviertassinari oliviertassinari added component: select This is the name of the generic UI component, not the React module! and removed component: text field This is the name of the generic UI component, not the React module! labels Jul 15, 2020
@oliviertassinari oliviertassinari changed the title Tree-Shaking does not work (minimal example) [SelectField] Introduce new component Jul 15, 2020
@oliviertassinari oliviertassinari added new feature New feature or request and removed discussion labels Jul 15, 2020
@oliviertassinari oliviertassinari added this to the v5 milestone Jul 15, 2020
@eps1lon eps1lon self-assigned this Jul 15, 2020
@eps1lon eps1lon removed their assignment Apr 19, 2021
@lovelle-cardoso
Copy link

lovelle-cardoso commented Aug 4, 2021

Our team was also really caught off guard by this. We noticed the bundle sizes for our form pages were really bloated despite us only using text fields and buttons from material-ui. Spent ages trying to track down where the bundler was getting confused and importing Selects and Modals, only to find that TextField includes Select, Modal, and their many dependencies by default.

For any other devs stumbling across this issue, here's a bandaid solution that you can use to greatly reduce the size of TextFields while material-ui works on an official fix. This custom component simply follows the structure of material-ui's TextField component but removes any references to Select and requires that the variant component is provided as a prop (This way, only the variant type you use will be included in your bundle, rather than all 3 standard, outlined, and filled variant components being included by default):

ComposableTextField.tsx

import React from "react";
import { InputProps } from "@material-ui/core/Input";
import { TextFieldProps } from "@material-ui/core/TextField";
import FormControl from "@material-ui/core/FormControl";
import InputLabel from "@material-ui/core/InputLabel";
import FormHelperText from "@material-ui/core/FormHelperText";

interface ComposableTextFieldProps
  extends Omit<TextFieldProps, "select" | "SelectProps" | "children"> {
  InputComponent: React.ComponentType<InputProps>;
}

const ComposableTextField = React.forwardRef(
  (props: ComposableTextFieldProps, ref: React.Ref<HTMLDivElement>): JSX.Element => {
    const { InputComponent, ...textFieldProps } = props;
    const {
      autoComplete,
      autoFocus = false,
      className,
      color = "primary",
      defaultValue,
      disabled = false,
      error = false,
      FormHelperTextProps,
      fullWidth = false,
      helperText,
      id,
      InputLabelProps,
      inputProps,
      InputProps,
      inputRef,
      label,
      maxRows,
      minRows,
      multiline = false,
      name,
      onBlur,
      onChange,
      onFocus,
      placeholder,
      required = false,
      rows,
      type,
      value,
      variant = "outlined",
      ...other
    } = textFieldProps;

    if (!InputComponent) {
      return null;
    }

    const InputMore: Record<string, unknown> = {};

    if (variant === "outlined") {
      if (InputLabelProps && typeof InputLabelProps.shrink !== "undefined") {
        InputMore.notched = InputLabelProps.shrink;
      }
      if (label) {
        const displayRequired = InputLabelProps?.required ?? required;
        InputMore.label = (
          <>
            {label}
            {displayRequired && "\u00a0*"}
          </>
        );
      }
    }

    const helperTextId = helperText && id ? `${id}-helper-text` : undefined;
    const inputLabelId = label && id ? `${id}-label` : undefined;

    return (
      <FormControl
        className={["MuiTextField-root", className].filter(Boolean).join(" ")}
        disabled={disabled}
        error={error}
        fullWidth={fullWidth}
        ref={ref}
        required={required}
        color={color}
        variant={variant}
        {...other}
      >
        {label && (
          <InputLabel htmlFor={id} id={inputLabelId} {...InputLabelProps}>
            {label}
          </InputLabel>
        )}
        <InputComponent
          aria-describedby={helperTextId}
          autoComplete={autoComplete}
          autoFocus={autoFocus}
          defaultValue={defaultValue}
          fullWidth={fullWidth}
          multiline={multiline}
          name={name}
          rows={rows}
          maxRows={maxRows}
          minRows={minRows}
          type={type}
          value={value}
          id={id}
          inputRef={inputRef}
          onBlur={onBlur}
          onChange={onChange}
          onFocus={onFocus}
          placeholder={placeholder}
          inputProps={inputProps}
          {...InputMore}
          {...InputProps}
        />
        {helperText && (
          <FormHelperText id={helperTextId} {...FormHelperTextProps}>
            {helperText}
          </FormHelperText>
        )}
      </FormControl>
    );
  }
);

export default ComposableTextField;

Simply replace any references to TextField with this ComposableTextField component instead. And make sure to pass in the InputComponent variant you'd like to use:

Replace this:

<TextField label="Outlined" variant="outlined" />

With this:

<ComposableTextField label="Outlined" variant="outlined" InputComponent={OutlinedInput} />

As a side note, the material-ui Text Fields documentation does mention that you can use the lower level components to better customize your TextFields, but perhaps it would also be worth mentioning that you can use these lower level components to reduce your bundle size in the Minimizing Your Bundle Size documentation.

@mj12albert
Copy link
Member

mj12albert commented Oct 19, 2023

Added this to the v6 milestone, most of the heavy lifting seems done in #38374 and #38972 which are already in progress CC @DiegoAndai

@mj12albert mj12albert changed the title [SelectField] Introduce new component [SelectField][material-next] Introduce new component Nov 3, 2023
@DiegoAndai DiegoAndai removed the v6.x label Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change component: select This is the name of the generic UI component, not the React module! performance
Projects
Status: Backlog
Development

No branches or pull requests

6 participants