Skip to content

Conversation

@mnajdova
Copy link
Member

@mnajdova mnajdova commented Oct 22, 2020

This PR prepares unnecessary changes for introducing the @material-ui/unstyled package. It moves the @material-ui/core/utils utilities to @material-ui/utils, so that those can be reused in the new package.

Exception from this rule is the createSvgIcon utility that was moved to '@material-ui/core/SvgIcon' because it depends on the SvgIcon component.

As part of the PR there is a codemod prepared (heavily inspired by the optimal-imports codemod), that basically replaces the old imports from @material-ui/core/utils to @material-ui/utils.

Example:

import React from 'react';
import { refType, HTMLElementType } from '@material-ui/utils';
import { capitalize } from '@material-ui/core/utils';
import { ownerDocument, isMuiElement } from '@material-ui/core/utils';
import { createSvgIcon } from '@material-ui/core/utils';

will be transformed to

import React from 'react';
import { refType, HTMLElementType, capitalize, ownerDocument, isMuiElement } from '@material-ui/utils';
import { createSvgIcon } from '@material-ui/core/SvgIcon';

The netlify build is failing because of the following:

3:26:54 PM: Failed to compile.
3:26:54 PM: 
3:26:54 PM: ModuleNotFoundError: Module not found: Error: Can't resolve '../../../../packages/material-ui/src/utils' in '/opt/build/repo/node_modules/@material-ui/pickers/dist'
3:26:54 PM: > Build error occurred
3:26:54 PM: Error: > Build failed because of webpack errors
3:26:54 PM:     at build (/opt/build/repo/node_modules/next/dist/build/index.js:15:918)
3:26:54 PM:     at process._tickCallback (internal/process/next_tick.js:68:7)
3:26:54 PM: error Command failed with exit code 1.
3:26:54 PM: info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
3:26:54 PM: error Command failed.
3:26:54 PM: Exit code: 1

I am open to ideas how to fix this.

@mui-pr-bot
Copy link

mui-pr-bot commented Oct 22, 2020

@material-ui/core: parsed: -0.21% 😍, gzip: -0.02% 😍
@material-ui/utils: parsed: +68.97% , gzip: +64.92%

Details of bundle changes

Generated by 🚫 dangerJS against 2bef269

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Oct 22, 2020
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Oct 23, 2020
@mnajdova mnajdova changed the title [WIP][utils] Move @material-ui/core/utils to @material-ui/utils [utils] Move @material-ui/core/utils to @material-ui/utils Oct 23, 2020
@mnajdova mnajdova marked this pull request as ready for review October 23, 2020 13:32
@mnajdova
Copy link
Member Author

I really am sorry about the +190 files changed 😢 but most of them should be just imports changes, so hopefully it won't be too hard for reviewing...

import Avatar from '@material-ui/core/Avatar';
import { withStyles } from '@material-ui/core/styles';
import { capitalize } from '@material-ui/core/utils';
import { capitalize } from '@material-ui/utils';
Copy link
Member

@oliviertassinari oliviertassinari Oct 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about keeping the same imports as before but simply having core re-export utils? It would:

  • remove the need for a codemod
  • remove the need for developers to import from the utils package
  • solve the release fail (true until we finish the migration of pickers in the lab)
  • reduce the git diff
Suggested change
import { capitalize } from '@material-ui/utils';
import { capitalize } from '@material-ui/core/utils';

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think that is much safer option :) will probably open new PR with only these changes

@oliviertassinari oliviertassinari added the package: utils Specific to the utils package. label Oct 24, 2020
@mnajdova
Copy link
Member Author

Closing in favor of #23264 as proposed in #23203 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: utils Specific to the utils package.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants