threepointone/glamor

Option to have glamor prepend it's style tag to the head (instead of append)

Closed this issue · 6 comments

jeroenransijn commented

In our app we use glamor alongside webpack with CSS Modules. In dev the loading of the CSS modules is pretty random, based on the way you load components.

The head might look like this in dev:

<head>
<title></title>
<style></style>
<style data-glamor>.box { margin: 0; }</style>
<style></style>
<style>.box { margin: 10px; }</style>
<style></style>
</head>

In production all the styles are put into a rendered in a singular stylesheet and loaded before:

<head>
<title></title>
<link href='bundle.css' />
<style data-glamor>.box { margin: 0; }</style>
</head>

Now the specificity is messed up.

A potential solution to this is to have glamor prepend it's style tag to the head:

<head>
<style data-glamor></style>
<title></title>
</head>

I am still on v2, wouldn't mind making the change to v2. But not sure how soon v3 is going to be released.

ChristopherBiscardi commented

Presumably, this is aggravated because of using style loader in dev vs ExtractTextPlugin in prod builds?

While I understand the potential issue, I'd like to see a concrete example if you don't mind supplying one. Is this related to combining glamor classes and cssmodules classes on the same element? I can't imagine a case in which the classnames collide (as in the example above), but perhaps you've implemented a custom CSSModules hashing algo?

threepointone commented

This is quite the edge case, aggravated because you're using glamor to inject global styles. Not sure we can (or should) do much here.

jeroenransijn commented

I can explain a little bit more about our use case. We are building a ui library outside of out web apps, and importing that library in our web apps. Since these web apps have different environments and css resets, we add a reset class (instead of global styles) to all of our components in the ui library. This makes sure that a component looks identical in different environments. So far this turned out to be very powerful. This are not global styles.

This reset class contains your basic reset: margin: 0; padding: 0; border: 0; amongst other things.

In our apps we have situations in which we might want to add a className to our components and add some specific styling that is not defined in our ui library component. The issue here is strongly related to style loader in dev vs ExtractTextPlugin in prod — but potentially spans multiple advanced configurations. In our case we would like to see glamor added before everything else, so we can overwrite without adding specificity. Example:

import { Box } from 'ui-library'
import css from 'module.css'

<Box className={css.specialBox} />

In this case we might have something like this in module.css:

.specialBox {
  margin: 20px;
  padding: 20px;
}

In dev the specialBox class might be defined after the glamor style tag. In production the ExtractTextPlugin consolidates all the css modules in one and creates a stylesheet and renders it before glamor. This means in production the reset styles might overwrite our defined styles.

in dev:

screen shot 2017-04-07 at 10 30 09 am

In production:

screen shot 2017-04-07 at 10 17 23 am

Potential ways to approach this:

  1. Manually add a <style data-glamor></style>. Have glamor look for this before creating a new one.
  2. An option for prepend instead of append. This would mean it is considered the base style in any scenario
  3. Do something in style-loader instead

I am a big fan and advocate of glamor. I am not here to force this technology to work for our specific use case. However, I do think when people start implementing glamor in their existing apps, they have to combine it with other technologies. These are real use cases, and some of the combinations are going to be really common. I think this combination is unfortunate but might be able to be fixed with additional configuration inside of glamor. In the end I can only see this drive to wider adoption by the community if glamor understands and answers to some common use cases. I hope we can figure something out that makes glamor better for all.

threepointone commented

Fair points. Will consider.

kentcdodds commented

Just driving by and noticed this issue. Here's how I solved the problem with a plugin called increase-specificity.js :)

/*
 * This plugin for glamor will prepend #id to the beginning of each selector
 * which glamor generates. So a selector like `.css-123abc, [data-css-123abc]`
 * will be changed to: `#id .css-123abc, #id [data-css-123abc]`.
 * This gives the selector higher specificity and should make it impervious
 * to other stylesheets on the page (component styles should always win a
 * conflict war with stylesheets).
 */
export default getSpecificityPlugin

/**
 * Gets the plugin function that uses the given id
 * @param {String} id the ID to prepend selectors with
 * @return {Function} the glamor plugin function
 */
function getSpecificityPlugin(id) {
	return specificityPlugin

	function specificityPlugin({ selector, style }) {
		selector = selector
			.split(',')
			.map(sel => `#${id} ${sel.trim()}`)
			.join(', ')
		return { selector, style }
	}
}

and the usage:

import { plugins } from 'glamor'
import getSpecificityPlugin from './increase-specificity'

// if the document.body.id doesn't exist, we'll make a random one and assign it
// then use that to increase our specificity
// making it random so it can't be used for anything other than this plugin.
document.body.id = document.body.id || `random-glamor-id-${Math.random().toString().slice(2)}`
plugins.add(getSpecificityPlugin(document.body.id))

And the unit test 😎

import getSpecificityPlugin from '../increase-specificity'

test('updates selectors to have the given ID', () => {
	const myId = 'abc-easy-as-123'
	const plugin = getSpecificityPlugin(myId)
	const input = {
		selector: '.css-234kjjdf, [data-css-234kjjdf]',
		style: { margin: 20 },
	}
	const result = plugin(input)
	expect(result).toEqual({
		selector: `#${myId} .css-234kjjdf, #${myId} [data-css-234kjjdf]`,
		style: input.style,
	})
})

I hope that helps! Should probably just open source this.

I should note that this makes the specificity pretty dang serious. aphrodite solves this problem by putting !important on everything (even more heavy handed). If you want to override styles globally for one reason or another, those overrides will most likely have to include !important to work. Either that or you have a deterministic id on the body and make them use that ID as well (and fight the specificity wars).

Good luck friends!

threepointone commented

The workaround is a good one. Another one is to manually move the style tag with a data-glamor attribute. Closing this.