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

"Shadow CSS" babel transformation #249

Closed
rauchg opened this issue Nov 13, 2016 · 26 comments
Closed

"Shadow CSS" babel transformation #249

rauchg opened this issue Nov 13, 2016 · 26 comments

Comments

@rauchg
Copy link
Member

rauchg commented Nov 13, 2016

When we land #26 and #21, it'll be interesting to explore a transformation that takes something like:

export default () => (
  <div>
    <p>woot</p>
    <css>{`
      p {
        color: red
      }
    `}</css>
  </div>
)

into

import Head from 'next/head'
export default () => (
  <div>
    <p className="__component-name_p">woot</p>
    <Head>
      <style id="__component-name_styles">{`
        .__component-name_p {
          color: red
        }
      `}</style>
    </Head>
  </div>
)

Some notes on the transpilation process:

  • We can use https://github.com/postcss/postcss-selector-parser to prefix I believe
  • Every JSX element needs to get a class for its tag name
  • Dynamic class names can be handled with a runtime-based prefixer
    <p className={ this.props.x ? 'y' : 'z' }</p>
    becomes
    <p className={__classNamePrefix(this.props.x ? 'y' : 'z')}</p>

Once browsers add support for server-rendered shadow CSS (#22), this is a transformation that would only be necessary for legacy browsers

@rauchg
Copy link
Member Author

rauchg commented Nov 16, 2016

Actually I just realized that it'll be much faster to use an approach for prefixing the CSS that does not involve parsing and creating an AST. Something like what buble does for ES6

@pemrouz
Copy link

pemrouz commented Nov 16, 2016

Could you make the "component-name" the tag name rather than a class? So the output would look something like:

<head>
  <style id="component-name-styles">component-name p { color: red }</style>
</head>
<component-name>
  <p></p>
</component-name>

You shouldn't need any runtime prefixers too this way (component-name .y and component-name .z will naturally match).

If you want to do more than prefix (upper bound isolation) and instead transform each rule set to a specific class (.__component-name_p), you'd have to parse AST (rebuild the browser CSS engine in JS rather) to see which elements match which selectors and then apply just those classes - on every render (hot path). For example, if you turn component-name > ul > li.is-selected > a into a single .__component-name_ul_li.is-selected_a, you'd have to check does this anchor tag have a selected parent? That would probably be overkill..

@rauchg
Copy link
Member Author

rauchg commented Nov 16, 2016

The idea of the component name prefix is to prevent collisions. Tag name is
not enough since you would have collisions across components or even with
nested components. Unless i'm missing some other point?

In fact I think the best would be {package.name}{component_name}_tag

On Wed, Nov 16, 2016 at 2:56 PM Pedram Emrouznejad notifications@github.com
wrote:

Could you make the "component-name" the tag name rather than a class? So
the output would look something like:

<style id="component-name-styles">component-name p { color: red }</style>

You shouldn't need any runtime prefixers too this way (component-name .y
and component-name .z will naturally match).

If you want to do more than prefix (upper bound isolation) and instead
transform each rule set to a specific class (.__component-name_p), you'd
have to parse AST (rebuild the browser CSS engine in JS rather) to see
which elements match which selectors and then apply just those classes - on
every render (hot path). For example, if you turn component-name > ul >
li.is-selected > a into a single .__component-name_ul_li.is-selected_a,
you'd have to check does this anchor tag have a selected parent? That would
probably be overkill..


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#249 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAAy8fdw3st2f1QaIAehZnZs9r6ooe90ks5q-4ougaJpZM4Kw1Sm
.

@kevinSuttle
Copy link

Something about this smells funny. Why not go all-in on Shadow DOM and not have to worry about classnames?

@rauchg
Copy link
Member Author

rauchg commented Nov 17, 2016

It's still not clear to me how to server-render shadow CSS (and what the backwards compatibility story is)

@kevinSuttle
Copy link

@addyosmani @ebidel ⬆️

@rauchg
Copy link
Member Author

rauchg commented Nov 17, 2016

It all comes down to this (h/t @treshugart for letting me know)
WICG/webcomponents#71

tl;DR there's no proposal yet for how this could be done, but there's preliminary interest.
I'm excited about the web platform gaining this capability. After all, one could even hand-roll isolated components with no frameworks or JS!

But I'm trying to think ahead. Even if a proposal is made, submitted, discussed, approved and rolled out in early-adopting Chrome, we'll need a fallback.

The best fallback I can think of for style isolation is described in the OP.

@pemrouz
Copy link

pemrouz commented Nov 22, 2016

The idea of the component name prefix is to prevent collisions. Tag name is
not enough since you would have collisions across components or even with
nested components. Unless i'm missing some other point?

Sorry wasn't very clear: I agree with component name prefixing. It's just a little easier to generate the fallback CSS and nicer when the component name prefix is the component-name tag name. In any case, this is perhaps more an issue for React than next.js: In the Custom Elements world, a component upgrades some <component-name> tag so you always have a clear root. React would need to change how it generates the markup first.

Tag name is not enough since you would have collisions across components or even with nested components

As far as I can tell, what you proposed won't solve this. Try running through a more complicated example: What would the output be if instead of p you had ul li.is-selected a? Component name prefixing will only solve upper bound isolation. There's no efficient way to automatically do lower bound isolation as far as I am aware. Perhaps @addyosmani @robdodson et al can clarify though what the latest thinking is in Polymer and if they've found a neater solution? It's noted on their homepage, but not sure and would love to know how they tackle/implement it..

@giuseppeg
Copy link
Contributor

Why not just keep the selectors as-is and bump their specificity of +0010 by adding a unique identifier class?

.foo {}
#hello {}
p a {}
h1 #foo [href] {}
* {}

becomes

/* ._aB1bC is unique for each component */

.foo._aB1bC {}
#hello._aB1bC {}
p._aB1bC a._aB1bC {}
h1._aB1bC #foo._aB1bC [href]._aB1bC {}
*._aB1bC {}

The css part is pretty straightforward, maybe patching the "dom" i.e. adding the class to each element is a bit more tricky but definitely doable.

To style the "shadow dom" from outside then we can provide the user with an helper that will patch its styles by adding the same class name:

<style for="Component">
   #hello { color: red }
</style>

becomes

<style>
   ._aB1bC-ext #hello._aB1bC {} { color: red }
</style>

Also should work for external files

<link rel="stylesheet" href="..." for="Component">

CSS as annotation?

/* @shadow^Component */
#hello {}
/* @shadow$ */

/* or */

/* <style for="Component"> */
#hello {}
/* </style> */

@threepointone
Copy link
Contributor

one thing to note is that

<css>
p { color: red }
</css>

is invalid jsx, since {} signals an interpolation to the parser. an alternative is to wrap it in a literal

<css>{`
p { color: red }
`}</css>

@rauchg
Copy link
Member Author

rauchg commented Nov 22, 2016

Great point. Still much better than the dangerous html attr 😇

On Tue, Nov 22, 2016 at 5:05 AM Sunil Pai notifications@github.com wrote:

one thing to note is that

p { color: red }

is invalid jsx, since {} signals an interpolation to the parser. an
alternative is to wrap it in a literal

{p { color: red }}


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#249 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAAy8WGYGX-qBumQ9bDNLVlHkDxiRgMZks5rAsyMgaJpZM4Kw1Sm
.

@kevinSuttle
Copy link

Just gonna leave this here: https://twitter.com/kevinSuttle/status/791326383234543618 :)

@ebidel
Copy link
Contributor

ebidel commented Nov 22, 2016

Polymer has separated out its shady dom css shimmer as a standalone lib: https://github.com/webcomponents/shadycss

The way that works is to prefix all nodes within the shadow tree with an element instance prefix (.my-element-1). That prevents lower-bound leakage. There's nothing that prevents upper-bound bleed-in. That requires native shadow dom.

@giuseppeg
Copy link
Contributor

giuseppeg commented Nov 22, 2016

There's nothing that prevents upper-bound bleed-in

@ebidel this is an hack but wouldn't it be enough to render the entire page/app/root component into shadow dom?

edit: I am referring to the use-case described above, where anything would use this pseudo-ShadowDOM. Obviously it is not possible with mixed DOM unless as you said we use native shadow dom.

@ebidel
Copy link
Contributor

ebidel commented Nov 22, 2016

@giuseppeg something like https://github.com/Polymer/shop/blob/master/index.html#L55? Yea, I suppose that could work, but it would require authors to use that pattern or a build/runtime solution to move things around :\

There would potentially still be main page styles that could leak into the app/root component.

@rauchg
Copy link
Member Author

rauchg commented Nov 26, 2016

@pemrouz what's the problem with .c-ul .c-li.c-is-selected .c-a ?

@rauchg
Copy link
Member Author

rauchg commented Nov 26, 2016

@pemrouz keep in mind that this technique involves a runtime classname prefixer as well, to support dynamic expressions

@rauchg
Copy link
Member Author

rauchg commented Nov 26, 2016

If you're talking about recursive nested components of the same kind, that'd be a collision, but since the styles are likely to be the same, probably not an issue. We never ran into that though

@giuseppeg
Copy link
Contributor

@rauchg I guess that converting selectors to classes can result in specificity bugs

@rauchg
Copy link
Member Author

rauchg commented Nov 27, 2016

By the way in conversation with @pemrouz we realized that we can avoid the class conversion step by adding an attribute to every node, and then rewriting the selectors to always target that attribute in addition to tag/class.

I think that addresses most problems!

@giuseppeg
Copy link
Contributor

@pemrouz
Copy link

pemrouz commented Nov 27, 2016

Hrmm, I guess that might work actually! :) I don't imagine there should be much runtime overhead either 👍. Wonder what Polymer folk think about this approach @ebidel @addyosmani? Specifically if it can be genericized to work independently of next? I guess this kinda works particularly well for you because of the templating layer (JSX), although this approach could definitely work separately too.

For recursive components, you're right. Probably shouldn't matter if they are the same component. But if you needed to, you could include an instance number in the prefix. But then just need to be conscious the styles tags in your head don't spiral out of control if you're creating a lot components. Probably best to avoid doing this until someone complains.

Few other points discussed offline further with @rauchg:

  • How would non-class selectors (e.g. attributes) work? Would you have to prefix them too?
  • Perhaps if we mark an element with who it's direct parent component is using an attribute, that would work better. Since it would avoid messing with className, there wouldn't need to be a runtime prefixer at all, just rewrite the styles. Non-class selectors would work too. Implementation simplicity aside, need some empirical data on the cost of runtime prefixer vs using attribute selector (which is probably slower than using a class).
  • This would need to be a babel transform so styles can be compiled AOT. However, it would be worth writing the core as a pure function similar to cssscope (given a stylesheet and component name, it returns the transformed stylesheet). That way it can be reused in more contexts (i.e. I can use this for Ripple :D)

@giuseppeg
Copy link
Contributor

giuseppeg commented Nov 27, 2016

Wonder what Polymer folk think about this approach @ebidel @addyosmani?

@ebidel explained that this is in fact how the Shady DOM CSS shimmer works.

How would non-class selectors (e.g. attributes) work? Would you have to prefix them too?

Imho anything should be suffixed with an unique class name, see my example.

Specifically if it can be genericized to work independently of next

This would be awesome!

Do you think that it is possible to split the transpilation in two phases?

The first one being patching the styles and returning some metadata with the unique stylesheet id (like CSS Modules do with classnames).

Then use those info to apply the given stylesheet id as classname to each html tag in the scope.

The benefit of splitting the transpilation in two phases is that we can have implementations of the second one in many languages I guess 🎉

@rauchg
Copy link
Member Author

rauchg commented Nov 27, 2016

I have an example in the works that's independent of Next. It'll work similarly to glamor or any other React-oriented styling system

@pemrouz
Copy link

pemrouz commented Dec 1, 2016

Sincere apologies @giuseppeg, missed your post 😞. What you posted was definitely based on the same concept..

@ebidel explained that this is in fact how the Shady DOM CSS shimmer works

I believe he confirmed my initial thoughts that lower bound encapsulation is not possible (see "There's nothing that prevents upper-bound bleed-in"). Which is also why I'm more curious as to what they think of this approach..

Do you think that it is possible to split the transpilation in two phases?

Definitely, styles can and should be transformed AOT.

I have an example in the works that's independent of Next. It'll work similarly to glamor or any other React-oriented styling system

Looking forward to this 👍

@rauchg
Copy link
Member Author

rauchg commented Dec 19, 2016

Done as a separate module styled-jsx that works independently of Next

@rauchg rauchg closed this as completed Dec 19, 2016
@lock lock bot locked as resolved and limited conversation to collaborators May 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants