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-cascade-5] Add a R/W CSSStyleSheet.layer attribute for the layer name #7002

Open
xiaochengh opened this issue Jan 31, 2022 · 19 comments
Open

Comments

@xiaochengh
Copy link
Contributor

xiaochengh commented Jan 31, 2022

(Edit: fixed some details on settings)

I'm proposing extending the CSSStyleSheet interface with a new attribute, layer.

On getting, it should return the layer name if the sheet is entirely in some cascade layer. In particular:

On setting:

  • If this is a constructed stylesheet and the new value is a non-null string, then the attribute value can be modified, and the stylesheet is treated as entirely in the given layer
  • If this is a constructed stylesheet and the new value is null, then the attribute value can be modified, and the stylesheet is treated as unlayered
  • Otherwise, throws a NotAllowedError DOMException

The primary use case is to allow adding layered constructed stylesheets, especially if the stylesheet is a 3rd party CSS module, and we want to use that to set up a web component (which we currently don't have a good way to make it layered):

import sheet from 3p-foo.css;

sheet.layer = 'new-layer';
shadowRoot.adoptedStylesheets = [sheet];
@tabatkins
Copy link
Member

Sounds good to me; this is an obvious hole currently both on the reading side (can't tell from inspection if a stylesheet was imported or linked with a layer) and on the writing side (can't have constructed sheets with the equivalent functionality of importing/linking into a particular layer).

@fantasai
Copy link
Collaborator

How should anonymous layers work? Maybe represent them with the empty string?

@xiaochengh
Copy link
Contributor Author

How should anonymous layers work? Maybe represent them with the empty string?

Yes. This is also consistent with how CSSImportRule.layerName distinguishes importing into an anonymous layer (empty string) vs. not importing into any layer (null).

@tabatkins
Copy link
Member

Ah, we should match that name, too, and use .layerName on CSSStyleSheet.

@tabatkins
Copy link
Member

(Or switch them both to .layer.)

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed Add a .layer attribute to CSSSTyleSheet, and agreed to the following:

  • RESOLVED: Add .layerName to CSSStyleSheet as a readonly attribute.
The full IRC log of that discussion <TabAtkins> Topic: Add a .layer attribute to CSSSTyleSheet
<TabAtkins> github: https://github.com//issues/7002
<TabAtkins> fantasai: So it just seemed reasonable to add, and discussion suggests it should be called .layerName to match the CSSLayerRule
<TabAtkins> astearns: And using .layerName is preferable to switching both to .layer?
<TabAtkins> fantasai: I'm open to both.
<smfr> prefer layerName
<TabAtkins> astearns: I think .layerName does suggest a string rather than an object, so it seems slightly more clear
<emilio> q+
<astearns> ack emilio
<TabAtkins> emilio: does @import allow us to set this on import, so we can make it immutable?
<fantasai_> TabAtkins: If this stylesheet is reflecting from a <link>, that's changeable in HTML
<fantasai_> TabAtkins: maybe we could make it immutable in the API?
<TabAtkins> emilio: Link mutations already have steps to recreate the stylesheet object
<TabAtkins> emilio: So that would trigger the same as changing media=''
<TabAtkins> emilio: Which creates a new stylesheet
<TabAtkins> TabAtkins: What about constructed stylesheets?
<TabAtkins> emilio: Hopefully we can set it in the constructor - it already has an option bag for things like baseURI, which also is immutable. Hopefully we can follow the same pattern.
<TabAtkins> TabAtkins: I'm fine with that.
<TabAtkins> emilio: The original issue uses @import syntax, which requires using an extra argument object somewhere to preserve the mime type...
<TabAtkins> TabAtkins: Don't understand
<TabAtkins> emilio: When you use importSheet()
<TabAtkins> scratch last line
<TabAtkins> emilio: When you use `import sheet from style.css`, can you set the layer there?
<TabAtkins> emilio: I think in JS you have a trailing object that lets you set the mimetype?
<TabAtkins> TabAtkins: Yes, but I'm unsure whether that's meant to be just assertions or can have extra data
<TabAtkins> astearns: Sounds like we can resolve on .layerName?
<TabAtkins> TabAtkins: And on makign it readonly unless that's killer for the JS ipmort syntax
<TabAtkins> emilio: Agreed
<TabAtkins> astearns: objections?
<TabAtkins> RESOLVED: Add .layerName to CSSStyleSheet as a readonly attribute.
<emeyer> scribenick emeyer

@xiaochengh
Copy link
Contributor Author

If it's a readonly attribute, how do we support the JS import syntax (which is the major use case I'd like to support)? We don't explicitly call its constructor so we can't pass the layer name in that way.

<TabAtkins> emilio: When you use import sheet from style.css, can you set the layer there?
<TabAtkins> emilio: I think in JS you have a trailing object that lets you set the mimetype?
<TabAtkins> TabAtkins: Yes, but I'm unsure whether that's meant to be just assertions or can have extra data

@tabatkins When you said "yes", were you talking about setting the layer or the mimetype?

@tabatkins
Copy link
Member

"yes" to trailing object; I just wasn't sure if it's meant to be able to carry additional data beyond the mime type.

@xiaochengh
Copy link
Contributor Author

Then the resolution is inadequate. We still need a way to make it work on the JS-imported style modules. The easiest way that I can see is still to make it writable on constructed stylesheets.

@emilio
Copy link
Collaborator

emilio commented Jun 3, 2022

@xiaochengh can't the trailing object carry the layer name? If not, why not?

@xiaochengh
Copy link
Contributor Author

Sorry I misunderstood. Didn't realize you were talking about import assertion. Yeah, we can pass a layer name there.

There's also one thing that I think we missed in the previous discussion. The same sheet can be imported for multiple times into different layers (e.g., when used by different modules), which means we need to make sure that each import creates a different CSSStyleSheet object. Is this already guaranteed, or do we need more work?

@mirisuzanne
Copy link
Contributor

Agenda+ to get @xiaochengh's question answered:

The same sheet can be imported for multiple times into different layers (e.g., when used by different modules), which means we need to make sure that each import creates a different CSSStyleSheet object. Is this already guaranteed, or do we need more work?

@fantasai and I are not clear on the answer.

@fantasai
Copy link
Collaborator

fantasai commented Aug 4, 2022

If CSSStyleSheet instances are not unique to their usage, and one can be imported in multiple places, then we can't have a layerName attribute. Instead, the importing mechanism needs to accept/track the layer name.

@css-meeting-bot
Copy link
Member

css-meeting-bot commented Sep 15, 2022

The CSS Working Group just discussed CSSStyleSheet.layer, and agreed to the following:

  • RESOLVED: No change to the previous resolution (for now, need more discussion)
The full IRC log of that discussion <emilio> Topic: CSSStyleSheet.layer
<emilio> github:https://github.com//issues/7002
<emilio> miriam: we resolved on the main question, a CSStyleSheet.layer
<emilio> ... another question came up that fantasai and me weren't sure how to answer
<emilio> ... can the same sheet be imported in different layers
<emilio> ... is it guaranteed that different imports end up with different sheets or do we need to track all the different names?
<Rossen_> q?
<emilio> q+
<Rossen_> ack emilio
<heycam> emilio: the CSSOM guarantees each @import rule should be a different style sheet
<fantasai> emilio: The CSSOM guarantees that each @import rule should be a different style sheet
<heycam> ... so I think we're good
<emilio> RESOLVED: No change to the previous resolution
<heycam> emilio: Xiaocheng is talking about CSS Imports, not @import
<heycam> ... I don't know how CSS Modules behave, if they always create the same style sheet object, or multiple
<heycam> ... I was talking about @import rules
<heycam> ... presumably if Xiaocheng is asking, they must only create a single object
<dbaron> not sure if https://github.com/WICG/webcomponents/blob/gh-pages/proposals/css-modules-v1-explainer.md is still accurate
<heycam> chrishtr: we should continue to research then get back to the issue
<heycam> dbaron: I have some memory of a discussion in a TAG review about whethehr they create separate objects, can't remember the conclusion
<dbaron> https://github.com/w3ctag/design-reviews/issues/405
<emilio> Rossen_: let's re-discuss when Xiaocheng is back

@chrishtr
Copy link
Contributor

@dandclark do you know the answer to @xiaochengh's question?

@zcorpan
Copy link
Member

zcorpan commented Sep 15, 2022

@atanassov atanassov removed the Agenda+ label Sep 15, 2022
@dbaron
Copy link
Member

dbaron commented Sep 15, 2022

Based on my memory from the CSS Modules TAG review (and a quick re-read to refresh my memory) in w3ctag/design-reviews#405, part of the idea of CSS modules is definitely that they are a new mechanism that allows the same stylesheet object to be referenced from multiple places, and then be used by adding to adoptedStyleSheets in multiple places. But it's not clear to me that there's any way to use a CSS module in a way that involves a layer name.

However, there was an issue related to @import (which I think is currently forbidden from CSS Modules) that was being deferred at the time of that TAG review. Depending on how that issue is resolved, it maybe be possible to use CSS @import syntax to extend the module graph, and thus create layer-name-specific imports that refer to sheets in the module graph. I'm not sure what eventually happened with that issue.

@dandclark
Copy link
Contributor

Separate imports of a CSSStyleSheet from a given URL will share the same stylesheet instance. This is analogous to how multiple imports of a given JS module reuse the same module.

That seems to be problematic for layerName; we'd need a way to guarantee that importing with a new layerName triggers a new instance of the sheet to be created, which isn't currently something supported by import assertions.

@dandclark
Copy link
Contributor

There's been discussion in the past about extending the import syntax to allow attributes that change things about the imported object (which is not allowed in the assert clause). This would create a new object for each import with different properties:

import stylesA from "./style1.css" assert { type: "css"} with {layerName: "foo"}; // Creates stylesheet instance
import stylesB from "./style1.css" assert { type: "css"} with {layerName: "foo"}; // Same instance as stylesA
import stylesC from "./style1.css" assert { type: "css"} with {layerName: "bar"}; // New instance because value in `with` clause is different

But this would require a syntax change going through TC39.

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