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

Class re-writing mode #27

Closed
rauchg opened this issue Dec 7, 2016 · 18 comments
Closed

Class re-writing mode #27

rauchg opened this issue Dec 7, 2016 · 18 comments

Comments

@rauchg
Copy link
Member

rauchg commented Dec 7, 2016

For background: #4 (comment)
The transformation we might have to use would look like this:

<div className={ a ? 'b' : 'c' }></div>

becomes

import _styledJsxPrefix from 'styled-jsx'
<div className=_styledJsxPrefix('r4nd0m', { a ? 'b' : 'c' })></div>

and the implementation of _styledJsxPrefix would perform class prefixing

@robhrt7
Copy link

robhrt7 commented Dec 12, 2016

Definitely looking forward to have such option, not only it will help excluding less performant attr styles, but also:

  • Keep selectors less specific
  • Output smaller generated CSS

@rauchg
Copy link
Member Author

rauchg commented Dec 19, 2016

Some elaboration on this.

If there's a tag selector:

p { }

it becomes

.$prefix_p { }

and each element gets a class with its tag name. ie: _styledJsxPrefix('$nonce', …, 'div')

If there's a class selector:

.woot { }

it becomes

.$prefix_woot { }

If there's an attribute selector:

[title=woot] {
  color: blue
}

it becomes

.$prefix[title=woot] {
  color: blue
}

and we make sure each element gets a generic random class as well, maybe by passing true as the last arg _styledJsxPrefix('$nonce', …, 'div', true)

@thysultan
Copy link
Contributor

thysultan commented Jan 4, 2017

@rauchg

The following will produce conflicts

.p {}
p {}
.$prefix_p { }
.$prefix_p {}

class and ids should probably use a different delimiter to differentiate it from tag selectors.

.$prefix_p { }  /* tag */
.$prefix__p {} /* class */

@giuseppeg
Copy link
Collaborator

The following will produce conflicts

also it breaks the specificity model.

The current behaviour is the right one because it bumps the specificity of every selector by 0010. Similarly a solution that involves the use of classes should add a class to every element.

className=_styledJsxConcat('h4sh', originalExpression)

Result:

p.h4sh {}
.p.h4sh {}
*.h4sh {}
#test.h4sh {}
/* and so on */

@thysultan
Copy link
Contributor

why is the hash after the user defined selector?

p {
color: red;
}

&:hover { color blue; }

to generate

.h4sh p { color:red; }
.h4sh:hover { color: red; }

and have the hash added to just the parent element

@giuseppeg
Copy link
Collaborator

.h4sh p will affect the entire subtree.

Also * { color: red } converted to .h4sh* { color: red } won't work

@rauchg
Copy link
Member Author

rauchg commented Jan 4, 2017 via email

@thysultan
Copy link
Contributor

thysultan commented Jan 4, 2017

@giuseppeg * { color: red } would actually be .h4sh * {} ... what do you mean by

affect the entire subtree

shouldn't the selector p { color: red; } affect every p element in the namespace'd element? where as > p { color: red; } will only affect the immediate descendants.

so

<div>
	<p>only this paragraph will get the style :)</p>
	<style jsx>{`
	  p {
	    color: red;
	  }
	`}</style>
</div>

becomes

<div class="h4sh">
	<p>only this paragraph will get the style :)</p>
</div>

then & { selects div.h4sh then p { selects the p in div.h4sh and * { affects everything in the div.h4sh.

@rauchg does that mean you also have to apply the namespace to the elements children, and how far down the tree does this happen if that's the case.

@rauchg
Copy link
Member Author

rauchg commented Jan 4, 2017

"affect the entire subtree" means that you're affecting elements of children components you don't know about

@rauchg
Copy link
Member Author

rauchg commented Jan 4, 2017

Obviously you could force people to use > but the whole point is to invert that model, and disallow access to subtress unless explicitly (:global())

@thysultan
Copy link
Contributor

You could auto insert the > otherwise you can run into cases where the output css and html is are large when using multiple selectors since every selector is getting a suffix and every element that matters is getting a class i.e

h1 span, h2 {}

will generate.

h1[data-jsx="woot"] span[data-jsx="woot"], h2[data-jsx="woot"] {}

@giuseppeg
Copy link
Collaborator

@thysultan size is not a problem gzip will do the magic since it is very repetitive data.

affect the entire subtree

I meant that in styled-jsx styles shouldn't cross the boundaries of the current component and therefore don't leak into nested components.

If you use the children selector > you'd have to reconstruct the dom structure to make sure that selectors match the right element(s). Imagine you have this dom

<div>
  <p>
    <span>zeit <span>rocks</span></span>
  </p>
</div>

and want to style the innermost span

span > span { color: red }

If you need to style it with the children selector the css should be:

div > p > span > span { color: red }

How would you do that? Also now you've increased the specificity of the selector.

@giuseppeg
Copy link
Collaborator

see #247

@yxh10
Copy link

yxh10 commented Sep 29, 2018

Could someone please explain how this resolves the styled-jsx not working with antd components issue?

I still could not get styled-jsx working with antd. antd components' class name doesn't have the random number generated by style-jsx. Therefore, it never matches the style.

The workaround I have are:

  1. modify the less file which affects the whole site if I don't add style contexts.
  2. inline styling

I think either of the solutions is not ideal. Would be great to have a consistent solution across the whole project which is having styled-jsx to handle all the styling.

Thank you very much for any help in advance.

screen shot 2018-09-30 at 11 38 11 am

@guanpengchn
Copy link

@yxh10 Do you have any idea about your question? I have the same problem now.

@tbarkley
Copy link

@yxh10 I'm also having this same issue

@bryjch
Copy link

bryjch commented Apr 1, 2020

In case anyone is dealing with this problem in the future, seems it can now be handled by either using:

I feel that using :global() is slightly preferred because it can have standard jsx structure:

import { Layout } from 'antd'
const { Header, Content } = Layout

<Layout id="app">
  <Header id="header">
    ...
  </Header>

  <Content id="content">
    ...
  </Content>
</Layout>

<style jsx>{`
  :global(#app) { ... }
  :global(#header) { ... }
  :global(#content) { ... }
`}</style>

I guess as long as the components have sufficiently unique ids/classNames to prevent css conflicts, it shouldn't be a big problem.

@toFrankie
Copy link

Could someone please explain how this resolves the styled-jsx not working with antd components issue?

I still could not get styled-jsx working with antd. antd components' class name doesn't have the random number generated by style-jsx. Therefore, it never matches the style.

The workaround I have are:

  1. modify the less file which affects the whole site if I don't add style contexts.
  2. inline styling

I think either of the solutions is not ideal. Would be great to have a consistent solution across the whole project which is having styled-jsx to handle all the styling.

Thank you very much for any help in advance.

screen shot 2018-09-30 at 11 38 11 am

除了 resolve tag 有什么更为优雅的解决方案吗?

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

No branches or pull requests

9 participants