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

Typography Migration Error on TextField #13175

Closed
charlie632 opened this issue Oct 9, 2018 · 26 comments
Closed

Typography Migration Error on TextField #13175

charlie632 opened this issue Oct 9, 2018 · 26 comments
Assignees
Labels
component: Typography The React component.

Comments

@charlie632
Copy link

Current Behavior

Some warning about migrating to Typagraphy V2 appeared on my console. I tried to resolve most of them, but an error appears everytime I type on a TextField. The error is as follows:

index.js:2178 Warning: Material-UI: You are using the typography variant body1 which will be restyled in the next major release. Please read the migration guide under https://material-ui.com/style/typography#migration-to-typography-v2

image

Tech Version
Material-UI v3.2.0
@eps1lon
Copy link
Member

eps1lon commented Oct 9, 2018

How did you try to resolve them? Have you read https://material-ui.com/style/typography/#migration-to-typography-v2?

@oliviertassinari
Copy link
Member

I think that we could improve the current situation in two ways:

  1. We could only raise once per variant.
  2. We could try to highlight the component stack in the warning.

@charlie632
Copy link
Author

How did you try to resolve them? Have you read https://material-ui.com/style/typography/#migration-to-typography-v2?

I had a lot of Display# in my code, so I just did some search and replace to h#. All of the errors disappeared after I did that, but then they started to appear as soon as I typed into a TextField.

@kitfit-dave
Copy link
Contributor

kitfit-dave commented Oct 10, 2018

I am seeing this issue also (typing in a TextField - I don't see the same issue when typing in an Input), though I'm not directly using variants or Typography or Themes.
I also seeing the same warning just using a Radio component.
Is it possible that somewhere within material ui is not using the new variants?
(@oliviertassinari component stack in the warning would be super helpful)

@ablakey
Copy link

ablakey commented Oct 10, 2018

I'm seeing a lot of typography variant warnings for things I don't control. I updated all my uses of Typography, but I still get a ton of them, I think, as a result of Material UI components that internally use Typography.

Finding the source is difficult because the trace for these warnings just takes me into the react-dom.development.js dispatch calls and never reveals the original component calling render() that causes this.

@kitfit-dave
Copy link
Contributor

I see that withStyles calls createMuiTheme (to create the default) without specifying useNextVariants - could this be the issue?

@eps1lon
Copy link
Member

eps1lon commented Oct 10, 2018

Just removing deprecated variants is not enough. You have to use a theme in your app that enables the typography v2 theme like so:

const theme = createMuiTheme({
  typography: {
    useNextVariants: true,
  },
});

A more detailed guide is available under https://material-ui.com/style/typography/#migration-to-typography-v2.

Otherwise you would still get old styles for variants like body1 and body2 which could be considered a breaking change if we just immediately changed those styles.

@kitfit-dave
Copy link
Contributor

Isn't "you have to use a theme" a breaking change too?

@eps1lon
Copy link
Member

eps1lon commented Oct 10, 2018

@kitfit-dave You only have to use it if you want to use the new feature. The warnings are "harmless". Please don't immediately jump on me. I know that they don't seem harmless and in dev mode your app becomes basically not debuggable with all that noise. That's why we also added the MUI_SUPRRESS_DEPRECATION_WARNINGS via env variable escape hatch.

That's a decision we have to make. Do we enable deprecation warnings by default (which is recommended by semver) or not.

As @oliviertassinari said we can improve our warnings in general by providing component stack and rate limit those warnings but I fear the rate limit is another area where you can only make mistakes.

@davalapar
Copy link

Yes lol the warnings are annoying because they're in red and not in yellow.

The new typography look (especially variant='overline') look gangsta though, love it.

I used this for now to make them disappear, hopefully I won't forget to remove em on next package update:

  typography: {
    useNextVariants: true,
    suppressDeprecationWarnings: true,
  },

@kitfit-dave
Copy link
Contributor

kitfit-dave commented Oct 10, 2018

Fair enough. Did not intend to jump on you :) It just seemed odd to me that the lib itself would cause deprecation warnings. I see that you'd be in a hard place making the default use the new variants by default. I suppose then that a future major version change will add the useNextVariants to any internally created themes?

A solution that I could see being useful would be to suppress the warnings for themes that the lib is creating itself? As the end user can't really fix those... or maybe that is just in my case as I am just accepting the default themes. And as you have rightly identified, it basically makes the app non debuggable with all the warnings flying. (And I'd personally prefer that to wrapping my app in a theme provider and creating a theme for no other reason than silencing warnings)

@eps1lon
Copy link
Member

eps1lon commented Oct 10, 2018

@davalapar When using typography v2 you should not need to suppress the warnings because you are good to go for the next major concerning typography. There is only one bug in DialogContentText where warnings would still get logged. This is fixed in master and should be released soon.

@kitfit-dave What do you mean by not fixable? The guide offers two solutions. We warn because we will default to v2 in the next major which introduces changes in the style which can potentially break your apps layout. You don't have to fear some perf penalty for passing a theme. That is already done internally.

@kitfit-dave
Copy link
Contributor

kitfit-dave commented Oct 10, 2018

What I mean is, I am not currently using a theme. I would have to create one and pass it in with a provider. This seems like quite a big change just to silence the warnings. Perhaps it's not... Though it is a change that I would then revert once the internal themes are created with the new variants.

@andrispraulitis
Copy link

Pardon me but the typo migration page clearly states that body1 is not deprecated which is why I don't understand the warning.

New variants:
display4 => h1
display3 => h2
display2 => h3
display1 => h4
headline => h5
title => h6
subheading => subtitle1
body2 => body1
body1 (default) => body2 (default)

@eps1lon
Copy link
Member

eps1lon commented Oct 10, 2018

@andrispraulitis It will change its style. The warning should say that.

@ablakey
Copy link

ablakey commented Oct 10, 2018

@eps1lon first of all, thank you for your help with this. It's very much appreciated.

I think it would make sense to only raise these warnings once per variant. The warning basically says, "hey, this variant isn't supported anymore!" so it would, in my opinion, be fair to report it per-variant and not per-render.

Greatly appreciate the MUI_SUPPRESS_DEPRECATION_WARNINGS escape hatch. I've used that to suppress a constant stream of these warnings, all caused by internal workings of mui components that I do not control.

@ablakey
Copy link

ablakey commented Oct 10, 2018

Here's an example of how to make a custom Mui theme that passes in the override config:

import React from 'react';
import { render } from 'react-dom';
import { MuiThemeProvider, createMuiTheme } from '@material-ui/core/styles';
import Root from './Root';

const theme = createMuiTheme({
  typography: {
    useNextVariants: true,
    suppressDeprecationWarnings: true
  }
});

function App() {
  return (
    <MuiThemeProvider theme={theme}>
      <Root />
    </MuiThemeProvider>
  );
}

render(<App />, document.querySelector('#app'));

I found this to be the easiest way to address this issue and move on for now.

@SandyGifford
Copy link

I find it a little frustrating that restyle warnings and removal warnings are both under the same suppression flag. I'd like to be aware of any instances of future breaking changes, but I've already gone through my code and prepped it for restyle changes, I don't need to see those anymore, or wade through them to find actual problem code.

@oliviertassinari
Copy link
Member

We will make a v3.2.1 release this evening with a better upgrade path leveraging everybody feedback.

@SandyGifford
Copy link

I find it a little frustrating that restyle warnings and removal warnings are both under the same suppression flag. I'd like to be aware of any instances of future breaking changes, but I've already gone through my code and prepped it for restyle changes, I don't need to see those anymore, or wade through them to find actual problem code.

Using the useNextVariants flag fixed this for me - I missed that one. Makes sense that if I had moved my code forward already I could just flip that switch. Thanks for the info!

@brennancheung
Copy link

I followed the instructions for migration but apparently there is still something not migrated but I can't find it. I have greped my entire codebase for the old variants and am still getting the Material-UI: you are using the deprecated typography variants that will be removed error. I have even gone so far as to have my main App.js just return (<span>test</span>) so that is the only thing rendered on the page and am still getting the errors.

Is there any way the deprecation error can say where the problem is occurring? At this point, I'm at a loss as to what hasn't been migrated yet.

@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 14, 2018

@brennancheung Some tips:

  1. Make sure you are using the latest version. I have fixed some issue regarding the warnings since it was first introduced
  2. If you are creating your own theme, you need to set useNextVariants:true with createMuiTheme().

It should be enough.

@sajadghawami
Copy link

sajadghawami commented Dec 6, 2018

even though i changed all the variants to the new ones, and i set useNextVariants:true i am still getting that same error - how come?

Edit:

If anyone having the same issue, it gotta be like this:

const theme = createMuiTheme({
  typography: {
    useNextVariants: true,
  },
});

@kpervin
Copy link

kpervin commented Jan 11, 2019

I am using a theme currently, and I have typography: { useNextVariants: true} in there, but I am still getting warnings. I also have updated all of my dependencies and my personal implementations of Typography. I'm unsure what to do next to remove these?

EDIT: Looks like I had, at one point before fully committing to Material-UI, had used individual themes for components and had never removed them. This is what was causing the issue.

screen shot 2019-01-11 at 2 01 44 pm

@joshwooding
Copy link
Member

joshwooding commented Jan 11, 2019

@ShaggyKris We would need to see your code to help fix this. What does your theme look like? I would make sure there isn't another theme that you have missed @oliviertassinari's comment will help with this

@oliviertassinari
Copy link
Member

@ShaggyKris You can expand one of the error, you will see the stack trace. With that, you can find where the culprit is 🔍.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: Typography The React component.
Projects
None yet
Development

No branches or pull requests