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

jss styles are inserted at the bottom of <head /> #7944

Closed
rsolomon opened this issue Aug 29, 2017 · 33 comments
Closed

jss styles are inserted at the bottom of <head /> #7944

rsolomon opened this issue Aug 29, 2017 · 33 comments
Labels
docs Improvements or additions to the documentation

Comments

@rsolomon
Copy link
Contributor

rsolomon commented Aug 29, 2017

Problem description

The v1 documentation here states that, "The CSS injected by Material-UI to style a component has the lowest specificity possible as the is injected at the top of the ". However, I am observing locally that the styles are being injected at the bottom of . I do not have any custom configuration set for JSS, so I don't believe anything in my codebase would be causing the difference in behavior.

I'm trying to follow the JSS docs to customize the insertion point via a comment, but everything I do within jss.setup() seems to be overriden with a MUIThemProvider that usesmuiThemeProviderFactory.js, which I don't have direct access to.

The docs examples use getContext, but that seems like a lot of extra boilerplate just to set a CSS insertion point. Does anyone have any advice on how I can get the JSS styles to actually render at the bottom of the <head /> element?

I'm reporting this here instead of SO, because I believe that the documentation is incorrect, and could use some examples for what I would expect to be a common use case for MUI consumers that don't also rely on JSS as their primary styling solution.

Versions

  • Material-UI: 1.0.0-beta.6
  • React: 15.6.1
  • Browser: Chrome latest
@rsolomon
Copy link
Contributor Author

rsolomon commented Aug 29, 2017

Update: I'm able to get it working using react-jss:

const jss = create(preset());
jss.options.insertionPoint = 'insertion-point-jss';

<JssProvider jss={jss}>
  <MuiThemeProvider theme={theme}>{this.props.children}</MuiThemeProvider>
</JssProvider>

However, I now get console warnings such as:

warning.js?6327:36 Warning: Failed context type: Invalid context `64a55d578f856d258dc345b094a2a2b3` of type `Jss` supplied to `withStyles(FormControl)`, expected instance of `Jss`.

I believe it's because I use Lerna, which is hoisting the jss I've manually installed, but not the one installed by material-ui, since I am using 2 versions (0.19.0 and 1.0beta) for legacy purposes.

I should also note that this introduces a lot of cruft in the way of dependencies and boilerplace. It would be great if we could just declare the insertion point as an argument to MuiThemeProvider.

@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 29, 2017

The CSS injected by Material-UI to style a component has the lowest specificity possible as the is injected at the top of the

I have observed the opposite default behavior. So that sentence might be 100% wrong. I was expecting the opposite at the time I wrote it.

However, I am observing locally that the styles are being injected at the bottom of the head.

@kof Any reason for this default behavior? If it's not an issue on JSS side, then we are going to have to update the documentation to match the reality.

I'm trying to follow the JSS docs to customize the insertion point via a comment

Our documentation is doing so to handle an external algolia style specificity issue.

@kof
Copy link
Contributor

kof commented Aug 29, 2017

I think its a good idea to allow all jss options passed down from mui.

If no insertion point found, JSS will inject at the bottom, as we assume that JSS is used for user styles and some other styles like css normalize/reset should be applied before.

@oliviertassinari
Copy link
Member

If no insertion point found, JSS will inject at the bottom, as we assume that JSS is used for user styles and some other styles like css normalize/reset should be applied before.

This argument is convincing 👍, we need to update the documentation.

@oliviertassinari oliviertassinari added the docs Improvements or additions to the documentation label Aug 29, 2017
@oliviertassinari oliviertassinari self-assigned this Aug 29, 2017
@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 29, 2017

I believe it's because I use Lerna, which is hoisting the jss I've manually installed

Yes, this warning makes me think you have a duplicated JSS module. Aside from the warning. Does it work?

It would be great if we could just declare the insertion point as an argument to MuiThemeProvider.

I'm not sure it's something that would be wise to do. Hopefully, we want to merge react-jss and material-ui styling solution. MuiThemeProvider responsibility is the theme, not the styling implementation. It's the JssProvider how has this responsibility. I would rather say that we could make so JssProvider accepts an insertionPoint property. But it's out of the scope of this project.

@rsolomon
Copy link
Contributor Author

Yes, this warning makes me think you have a duplicated JSS module. Aside from the warning. Does it work?

Yep it does work.

I would rather say that we could make so JssProvider accepts an insertionPoint property.

I don't have a problem with that, but I will say that it doesn't seem ideal to require including another dependency explicitly just to change a flag.

@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 29, 2017

Yeah, we break the encapsulation as soon as users need to customize the default behavior, not great. I would encourage you to open an issue on the react-jss side with your use case. There isn't much anything special with Material-UI, I think that all react-jss users are affected by this issue.

@Undistraction
Copy link

Undistraction commented Sep 6, 2017

So given that the styles generated by JSS are injected at the end of the header, how do we configure JSS with a different insertion point?

The docs say:

If you are experiencing a CSS injection order issue, take a look at the mechanism JSS provides on how to handle this situation. By adjusting the placement of the insertionPoint comment within your HTML body you can control the order that the CSS rules are applied to your components.

But it doesn't look like you expose the instance of JSS you're using so there is no viable way to specify the insertion point when using Material UI.

@rsolomon
Copy link
Contributor Author

rsolomon commented Sep 6, 2017

@Undistraction I've unfortunately resorted to reconfiguring my other styling solution to insert below JSS, instead of trying to reconfigure JSS itself. The only way I found it possible to reconfigure the JSS insertion point was to manually import jss-react and reconfigure everything that already happens behind the scenes with MUI. It ended up being much easier to configure react-css-modules than keep fiddling with JSS.

@oliviertassinari
Copy link
Member

Here is an example of how the documentation is handling it:

@Undistraction
Copy link

@rsolomon Thanks for the suggestion, but unfortunately I have CSS Module styles that are being compiled by Webpack - they are in place before the application is initialised and JSS does its thing. So I need to be able to place the insertion point before the CSS generated by Webpack.

@Undistraction
Copy link

@oliviertassinari Thanks for the links.

@rsolomon
Copy link
Contributor Author

rsolomon commented Sep 6, 2017

I also compile CSS modules using extract css plugin. It creates a file that you should have complete control over where it's imported.

@Undistraction
Copy link

@rsolomon But if even if I place the CSS Modules import at the end of the head, JSS will run at runtime, long after the html page has already been generated, and will place its styles afterwards ... or am I missing something?

@rsolomon
Copy link
Contributor Author

rsolomon commented Sep 6, 2017

Top of <body /> :)

@Undistraction
Copy link

Undistraction commented Sep 6, 2017

@rsolomon Well that's something I didn't know was allowed, but looks like it was made valid in HTML5.

@rsolomon
Copy link
Contributor Author

rsolomon commented Sep 6, 2017

It's not ideal, but I found it to be a much more concise solution than recreating the entire JSS config and importing the JSS-react dependencies.

@Undistraction
Copy link

Yep. I imagine it will be a pretty common request, so a simple way to do it that is part of Material UI makes sense to me.

@oliviertassinari
Copy link
Member

Alright, I'm OK to diverge again from the react-jss projet. Maybe we could add a jssOptions property to the MuiThemeProvider.

@oliviertassinari oliviertassinari removed their assignment Sep 16, 2017
@trungtin
Copy link

trungtin commented Oct 5, 2017

So any update on this?
When I wrap MuiThemeProvider like this to set the insertionPoint:

<JssProvider jss={jss}>
  <MuiThemeProvider theme={theme} sheetsManager={new Map()}>
          ...
  </MuiThemeProvider>
</JssProvider>

On the client it'll generate wrong class name: root-0-7 for MuiPaper-root-8 (on Server)
And when set:

jss.options.createGenerateClassName = createGenerateClassName

It generated the correct classname like the server, but it has this warning:

Warning: Material-UI: You can only instantiate one class name generator on the client side.
If you do otherwise, you take the risk to have conflicting class names in production.

@oliviertassinari
Copy link
Member

@trungtin Are you able to figure out why multiple class name generators are created?

@trungtin
Copy link

trungtin commented Oct 6, 2017

I'm not familiar with JSS so have no idea where to start. Do you have any suggestion?
There is another problem when wrapping JssProvider on the client side:
I'm using universal-router, so when going from one route without MuiButton to another having MuiButton, the injected style seems to collide with existing styles:

<style type="text/css" data-jss="" data-meta="MuiAppBar">
.root-0-0 {
  ...
}
...
</style>

then this one is injected to the header

<style type="text/css" data-jss="" data-meta="MuiButton">
.root-0-0 {
  ...
}
...
</style>

Notice the same className
But if go directly to the route with MuiButton. Then the className will be generated differently, and thing work well

@kof
Copy link
Contributor

kof commented Oct 6, 2017

Right now JssProvider has to be on a very top and not be rerendered. This will be fixed in the next react-jss version.

@blainegarrett
Copy link
Contributor

Apologies if this is disinfo, but a few days ago I was getting this error installing the create react app example in the @next branch for the first time. All the dependencies were pinned at "latest". It seemed uninstalling the jss dependencies (pinned at latest) and then reinstalling only the jss-react and jss-preset-default seemed to work. My hunch was that the "latest" versions got out of sync and thus were generating the hash differently.

Currently, my pins are: "react-jss": "^7.2.0", "jss-preset-default": "^4.0.1"
Currently latest of react-jss has pins:
"jss": "^8.1.0",
"jss-preset-default": "^3.0.0",

The latest jss is version 9.0.0. My node fu isn't super strong, but if my understanding is correct when jss is manually pinned at 'latest' (9.0.0) and react-jss at "latest" (7.2.0) the later is expecting the 8.0.x release of jss.

@oliviertassinari
Copy link
Member

@blainegarrett Please follow #8717.

@SwinX
Copy link

SwinX commented Oct 24, 2017

Guys any update on how insertionPoint may be configured? Latest suggestion

<JssProvider jss={jss}>
  <MuiThemeProvider theme={theme} sheetsManager={new Map()}>
          ...
  </MuiThemeProvider>
</JssProvider>

is not working for me for some reason.

@ghost
Copy link

ghost commented Feb 27, 2019

I'm trying to change the insertion point for material-ui/core v1.5.1. Nothing I've tried has worked.

I'd really like to get rid of this JSS garbage in my react-admin project. The whole mess of code surrounding the management of JSS just stinks so badly. The team that makes JSS also puts their trashy advertisements in the packages postinstall hooks. They obviously don't respect developers and I want nothing to do with their tech.

Has anyone figured this out for v1.5.1?

@ghost
Copy link

ghost commented Feb 27, 2019

@kof Seriously...imagine if every package did that? (Here's another FU buddy - seeya around ;)

@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 27, 2019

I'm trying to change the insertion point for material-ui/core v1.5.1. Nothing I've tried has worked.

@waynebloss I'm working on improving the API for v4. It's a common use case, it should be trivial to do. You can find examples in https://material-ui.com/guides/interoperability/.

The team that makes JSS also puts their trashy advertisements in the packages postinstall hooks

I have already raised this concern to @kof. The post install was removed in JSS v10, the version Material-UI v4-alpha depends on.

@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 27, 2019

Seriously...imagine if every package did that?

@waynebloss Imagine if every package user was contributing more?

@ghost
Copy link

ghost commented Feb 27, 2019

@oliviertassinari Trashy advertising is self evident. You're looking at exactly one of my accounts.

Perhaps you should try knowing more before pointing fingers.

Gee, I do have 66 repos on this account tho. Does that not count as contributing?

@ghost
Copy link

ghost commented Feb 27, 2019

I'm guessing you also think reporting bugs isn't contributing too lol.

Hey everybody!! Stop reporting bugs to MUI, they don't need you. kthxbai!!

@kgregory
Copy link
Member

kgregory commented Feb 27, 2019

Perhaps you should try knowing more before pointing fingers.

@waynebloss Perhaps you should try a different approach when communicating with people who dedicate a large portion of their lives to something you benefit from without the expectation of payment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

No branches or pull requests

8 participants