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

[css-shadow-parts] consider moving part-mapping to style rules #2904

Closed
fergald opened this issue Jul 11, 2018 · 21 comments
Closed

[css-shadow-parts] consider moving part-mapping to style rules #2904

fergald opened this issue Jul 11, 2018 · 21 comments

Comments

@fergald
Copy link
Contributor

fergald commented Jul 11, 2018

This came up in a meeting with Salesforce. It seems reasonable to consider this option.

The current spec provides the partmap attribute to allow a part inside a sub-component to be stylable from outside the containing component. This means that forwarding rules are spread across many elements, e.g.

<x-bar>
  # shadow
  <x-foo partmap="inner outer">...</x-foo>
  <x-foo partmap="inner outer">...</x-foo>
  <x-buz partmap="buz-part">...</x-foo>
</x-bar>

An possible alternative to that would be to include the part-mapping information in the style rules. There are several ways we could do this e.g.

<x-bar>
  # shadow
  <style>
    /* this causes x-bar::part(outer) to match elements inside x-foo */
    x-foo::part(inner) { @outer-name(outer) } 
    /* expose buz-part without renaming */
    x-buz::part(buz-part) { @outer-name() } 
  </style>
  <x-foo>...</x-foo>
  <x-foo>...</x-foo>
</x-bar>

Alternative ways of expressing this could be e.g.

@partmap myMap::x-foo {
	inner => outer; 
}

x-foo {
    @partmap(inner, outer);
}

x-foo::part(inner) { outer-name: outer } 

Good points:

  • all mappings are centralized
    • although they may be spread through multiple style elements inside the shadow root, so not necessarily centralized
    • exposing the part name map via IDL is something we want, no matter how it expressed and anyone who wants to reason about the mapping should use that
  • can compress many repetitive partmaps into a single CSS rule
  • makes it even easier to emulate ::theme using ::part, each shadow tree can just do ::part(theme-name) { @outer-name()} to expose all the theme-name parts
  • could also be used to add part names to elements, e.g. .some-class { @outer-name(some-part) } would be the same as adding part="some-part" to all of the elements of class "some-class"

Possible objections:

  • all mappings are centralized
  • these are not style rules, so should not be in the style sheet
  • unclear if there are implementation problems with this

This is a fairly radical departure from the current spec and if people think this is a great/terrible idea, I'd like to hear that quickly. I think there's no difference in what can be expressed in either case, the main points are:

  • the ability to expressing mappings for many elements in a compact form
  • centralized vs distributed

[ Edited by @dbaron to change a file: URL to the https: URL for the spec. ]

@dbaron
Copy link
Member

dbaron commented Jul 11, 2018

My initial (and not particularly carefully considered) reaction is that it feels rather complex; it's a good bit of additional syntax both for implementors to implement and for developers to learn. If the value it provides is worth the complexity, then it seems worth considering, but it's not clear to me whether it is.

@upsuper
Copy link
Member

upsuper commented Jul 11, 2018

This seems like proposing to make selector matching depend on cascading result... It's unclear to me whether this setup is going to work.

@fergald
Copy link
Contributor Author

fergald commented Jul 11, 2018

@upsuper If you think of ::part(foo) { @outer-name() } as a style rule that sets a property on all matching elements as part of the cascade then yes. But there is no property being set. So you could just think of it as a way to specify the part mappings (that is clearer in some of the alternative versions where it's an @ rule) and it would not be implemented as anything like a normal style rule.

I can imagine that this idea could be quite objectionable itself. So maybe an @ rule that looks like

<style>
@partmap {
    /* this causes x-bar::part(outer) to match elements inside x-foo */
    x-foo::part(inner) => outer;
    /* expose buz-part without renaming */
    x-buz::part(buz-part);
}
</style>

@fergald
Copy link
Contributor Author

fergald commented Jul 11, 2018

@dbaron I think you actually have less custom syntax in some versions of this because it's just selectors and a pseudo-property "outer-name". You've gotten rid of the comma-separated partmap attribute entirely. However you do have some new and maybe unexpected semantics to deal with.

@idoros
Copy link

idoros commented Jul 11, 2018

Hey, we are developing a pre-processor called Stylable, which extends the idea of scoping stylesheets with the ability to expose a "style API" for inner parts & states. Improving developer experience with a static language server that provides completions and warnings for this kind of style API.

This topic is interesting to me, because although Stylable currently implements scoping by namespacing the output CSS in build time, it could potentially provide the same developer experience for styling Web-Components. and hopefully output CSS that is more similar to the source CSS, moving the scoping to the browser engine.

So currently Stylable treats any CSS class as a potential custom pseudo-element (similar to ::part()):

/* btn.st.css */
.label { color:red; }
/* panel.st.css */
:import {
    -st-from: './btn.st.css';
    -st-default: Btn; /* import Btn definition */
}
.okBtn {
    -st-extends: Btn; /* give okBtn a Btn interface */
}
.okBtn::label { color:green; } /* access the label */

Notice: Stylable source file contain custom syntax that is transpiled away (like :import, -st-extends and the access to the custom pseudo-element ::label).

Which will be transpiled into:

.btn__label { color:red; }
.panel__okBtn .btn__label { color:green; }

If css shadow parts will allow defining parts through CSS definition, it would help tools such as Stylable to provide tooling to enhance web components authoring. For example we could output the previous CSS for web components (with the .some-class { @outer-name(some-part) } suggestion):

/* btn.st.css */
.label { 
    @outer-name(label);
    color:red;
}
/* panel.st.css */
.okBtn::part(label) { color:green; }

@caridy
Copy link

caridy commented Jul 11, 2018

thanks @fergald for creating the issue. There are few more things from the meeting yesterday that I want to give more context on why we are proposing this:

  • ergonomics: creating a new IDL for the remap attribute seems very limiting, and also suffered from some of the issues mentioned above, e.g.: users will have to learn that new syntax no matter what. The only prior art that I can think of is the style attribute.
  • perf: it you move the remap info into a CSS, it is probably easier to cache the remapping (speculating here). At least you don't have to worry about the remap attribute reflection and mutation.
  • extensibility: in the CSS we have more room for experimentation with the rules (we have quotes, line breaks, etc).
  • equivalence: ultimately, users who want to have the remap in the place where they use the element can use the style attribute with the remap rules on it. (e.g.: <x-foo style="::part(inner) { @outer-name(outer) }">...</x-foo>

Yes, there are some cons:

  • you need at least one style blob to do any remapping (I don't think this is a big deal).
  • decentralization since the style blob might reference things that are not longer in use by the markup (counter argument is the usage of the style attribute)

@fergald
Copy link
Contributor Author

fergald commented Jul 17, 2018

@caridy I think your example in 'equivalence" is not correct, style= shouldn't be a selector-declaration pair, it should just be the inside of the declaration. So, it would have to be <x-foo style="@other-name(outer)"> which is not remapping, it's just naming, so equivalent to <x-foo part="outer">. To be able to do the remapping in-place you would need to go with the version where the inner and outer names are included inside the declaration, <x-foo style="@partmap(inner, outer)"> and when you use it in a stylesheet you select the host element without using ::part(inner).

@fergald
Copy link
Contributor Author

fergald commented Jul 17, 2018

If there is ever any future pure-HTML use case for and API like part, for exposing bits of shadow trees in a controlled way, then we should leave it as a purely HTML feature and not bring style sheets into it it.

@fergald
Copy link
Contributor Author

fergald commented Jul 18, 2018

@rniwa @domenic @tabatkins please take a look at this idea. I'm not a huge fan of it (having already implemented the existing spec). It seems a bit weird and is quite different to the existing proposal but it does seem to have some upsides and I don't see an immediate reason to dismiss it. It would be great to hear strong reasons one way or the other.

@sorvell
Copy link

sorvell commented Jul 25, 2018

This seems like a pretty powerful alternative to the current spec. It seems like it could be the exclusive way to describe parts. Consider:

// Defining Parts
// in x-foo
<style>
  .thumb.active {
    @part(thumb-active);
  }
</style>

// Forwarding Parts
// in x-host that contains an x-foo
<style>
  x-foo::part(thumb-active) {
    @part(x-foo-thumb-active);
  }
</style>

This would make it pretty easy to define parts that match complex selectors. For example, this would make it easier to expose parts for the "red" and "black" squares on a checkerboard.

If this is at all feasible to implement it seems worth exploring, at least briefly.

@rniwa
Copy link

rniwa commented Aug 1, 2018

I don't think we want to implement something this complicated. It would mean we'd need to go find these part-defining rules separately and run them at a completely different timing than when we actually apply styles. This would also make it very expensive to implement any IDL attribute for the mapping, which seems highly undesirable.

@fergald
Copy link
Contributor Author

fergald commented Aug 2, 2018 via email

@rniwa
Copy link

rniwa commented Aug 2, 2018

Manipulating rules is decidedly worse than exposing part or partmap IDL attribute for developer ergonomics.

This proposal is definitely a lot harder to implement in WebKit than the DOM-based alternative, and it would be quite a bit slower to run.

@sorvell
Copy link

sorvell commented Aug 2, 2018

The ergonomics are debatable but I do think the selector based version is more powerful.

However, I also think the attribute based version (which is obviously much father along) makes sense and is workable.

@justinfagnani
Copy link

I'm unsure of implementation costs, but after thinking about it a bit, defining parts in CSS is definitely more powerful, especially with pseudo-classes and elements because you can abstract away the pseudo-class from the outside.

Some examples I thought of:

Defining parts of a checkerboard:

.cell:nth-child(odd) {
  @outer-name(odd-cell);
}
.cell:nth-child(even) {
  @outer-name(even-cell);
}

Disabling arbitrary elements and inputs:

:disabled, [disabled] {
  @outer-name(disabled);
}

Adding part names for light children and shadow children via ::slotted():

::slotted(.item), .item {
  @outer-name(item);
}

(Think of this when combined with display: contents on the host to treat light and shadow children as siblings for flex and grid layouts. It be nice to give a single part name rather than have the outside select the parts and children separately.)

Exposing a pseudo-element like ::before

li::before {
  @outer-name(marker);
  content: counter(li);
}

I'm sure there's some other cases where a component wants hover, active, etc states abstracted or styled the same as some other parts.

@rniwa
Copy link

rniwa commented Aug 3, 2018

The problem is that the power comes with a cost. Being able to change the presence of a part based on :hover, etc... would require a new mechanism to keep track of those states separately from actual style changes. Style invalidation would be a nightmare because now we'd need to first resolve part selectors to see if any part names have changed, then schedule a secondary path to actually invalidate elements, etc...

Honestly, if we picked this syntax, we'd probably never get around to implement this feature.

@idoros
Copy link

idoros commented Aug 3, 2018

What about simplifying it to work only for classes or ids?

@rniwa
Copy link

rniwa commented Aug 3, 2018

If we did that, what's the point of doing this in CSS? Also, we'd still have to keep matching those IDs & classes to find out if any given element defines a part or not.

Here's another problem. If we did this in CSS, I bet my money on someone coming around and asking for a DOM event whenever the presence of a part changes (i.e. appears / disappears). We won't be able to implement that because it would basically involve browser engines constantly evaluating CSS selectors which define parts to decide whether any given DOM mutation should result in the firing of such an event.

@idoros
Copy link

idoros commented Aug 5, 2018

what's the point of doing this in CSS?

I wrote my case above. Not having the ability to define the parts in CSS, means that my users will have to maintain a more complicated source, or compromise on custom run-time code in order to support web components.

we'd still have to keep matching those IDs & classes to find out if any given element defines a part or not.

I agree, this is another level of abstraction between CSS an DOM.

Here's another problem. If we did this in CSS, I bet my money on someone coming around and asking for a DOM event whenever the presence of a part changes (i.e. appears / disappears).

I don't know the implementation internals that you are, obviously, aware of, but I think that there are lots of use cases where allowing the developer to hook into changes in the CSS with JavaScript will improve the way we build web applications today.

also:

  • Wouldn't such a request might already be raised for components wanting to know when a composed component changed it's parts?
  • Isn't this already the case with animationstart and transitionstart ?

That being said, I accept that you have more knowledge of the implementation risks. And if this is an implementation / performance issue, then it's beyond my current understanding. However, API-wise, I still think having the parts definitions as part of the style, which is also the context in which a developer targets these parts, is better.

@rniwa
Copy link

rniwa commented Aug 5, 2018

Wouldn't such a request might already be raised for components wanting to know when a composed component changed it's parts?

That can be easily done if this were DOM API because then detecting any change to whether a given element is exposed as a part or not is very fast & easy. Detecting whether a arbitrary set of selectors would match or cease to match an element is an extremely expensive operation. This is why modern browser engines match selectors lazily when updating the styles.

Isn't this already the case with animationstart and transitionstart?

animationstart and transitionstart are different because they're async events, and the browser already has to know when an animation or a transition starts (in order to start the animation or the transition). Also, CSS animations doesn't start until the next painting / rendering happens, and the browser engine already has to update the style in those cases so detecting whether a given element has an animation or not is virtually cost-free.

@tabatkins
Copy link
Member

We're generally fine with dropping this idea. Vast majority of part mappings will be static and thus easily inlineable in the first place; the tiny fraction that aren't can be done with mutation observers or some other state-tracking. On the other hand, Ryosuke's speed concerns are definitely legitimate, plus weirdness like making slot-ness depend on a :hover rule or something.

So, closing.

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