search

WICG / construct-stylesheets

API for constructing CSS stylesheet objects
Other
138 stars 25 forks source link

Consider a different name for 'adoptedStyleSheets' #97

Closed Rich-Harris closed 9 months ago

Rich-Harris commented 5 years ago

The name adoptedStyleSheets is a bit awkward, for a few reasons:

It's long

Unwieldy names aren't uncommon in the DOM, but it's particularly problematic when the most common example usage...

document.adoptedStyleSheets = [...document.adoptedStyleSheets, newSheet];

...involves repetition. (I'm hopeful that this pattern will be abandoned in favour of explicit methods, due to the fact that it will result in nasty race condition bugs, but it's still a lot to type.)

'Adopted' is confusing

I'm not aware of any other places in the DOM where this verb is used. The most commonly understood meaning of 'adopted' is in the family context, where there's a parent-child relationship. Multiple families cannot adopt the same child. Used here, it could easily be understood to mean that a stylesheet can belong to a single element.

The casing is hard to remember

Although CSS stands for Cascading Style Sheets, people informally talk about 'stylesheets' rather than 'style sheets' — one word. You only ever see <link rel="stylesheet">, never <link rel="styleSheet">. This makes it harder to remember whether it's adoptedStylesheets or adoptedStyleSheets, particularly if you're not writing it frequently.

In fact, this is part of a spec called Constructable Stylesheets!

Of course, there is an easy way to remember it, but...

...The acronym is unfortunate

Not that I'm opposed to levity in web development, but I guarantee that this will be universally shortened to 'document dot aSS' or 'this dot aSS'. I'm not sure if that's what everyone had in mind.

Is there a reason this couldn't simply be document.css? An API like this would be a lot more palatable to most developers:

document.css.add(sheet);

(I realise there are concerns about being able to control the order of the collection, so add and remove may be too simplistic an API, but I'd expect appending to account for 99% of cases. Regardless, that's a separate issue.)

dtruffaut commented 5 years ago

I like it.

It mimics the KISS paradigm of :

e.classList.add('...')

... which I love to use.

In webcomponents, I probably would like to do :

this.css.add(sheet);

To add global stylesheets, I might do :

globalThis.css.add(sheet);
matthewp commented 5 years ago

'Adopted' is confusing I'm not aware of any other places in the DOM where this verb is used.

It's defined here and can be seen in a few APIs such as document.adoptNode and the CE callback adoptedCallback.

Rich-Harris commented 5 years ago

Step 2 from that spec page:

If node’s parent is not null, remove node from its parent.

In other words, a node cannot be adopted by multiple parents. It feels like it's being used incorrectly here.

zcorpan commented 5 years ago

There is a namespace object named CSS, though I think it's more for utility functions like CSS.escape(s) rather than managing stylesheets. Just something to keep in mind to not end up with a confusing API, like if there's globalThis.css and globalThis.CSS.

https://drafts.csswg.org/cssom/#utility-apis

calebdwilliams commented 5 years ago

Wouldn't globalThis.css not work since the adoptedStyleSheets currently sits on DocumentOrShadowRoot? I agree we should be weary of collisions, but this fact might mitigate your concern.

Lonniebiz commented 5 years ago

One thing about "css", is that it specifies the exact technology used today for style sheets. If there were to ever be another styling type, other than CSS (that could be mixed into this array along with css), at that point "css" would become a bad name.

I like "styles" (plural) for the name because it reminds me that this is an array of styles even if [onlyOneStyleIsUsed] and the list could potential hold any future technology-type for styling if that ever happens. "styles" is also pretty short (to type) compared to "adoptedStyleSheets". "styleSheets" also comes to mind too as a better choice. "sheets" is another option. "adopted" needs to be removed at the very least.

justinfagnani commented 5 years ago

We definitely need some array-like structure for the API, since the underlying semantics are array based. I see the need for these operations:

calebdwilliams commented 5 years ago

For what it's worth, I think the initial idea was to use moreStyleSheets to draw a parallel with DocumentOrShadowRoot.prototype.styleSheets. I just want to make sure this fact wasn't overlooked, the initial goal was to maintain an association between the two property names. I definitely see value in that, but also realize it isn't strictly necessary, but wanted to make sure that point isn't overlooked moving forward.

tabatkins commented 5 years ago

The casing is hard to remember

It's the exact same casing already used in document.styleSheets. I agree it's confusing and I don't like it, but it would be much worse to adopt something different.

'Adopted' is confusing

Legit. Our usage of 'adopted' elsewhere in the platform does indeed imply ownership that's not present here. My original suggestion was, iirc, .moreStyleSheets, which wasn't great either. ^_^

...The acronym is unfortunate

lol, good point

It's long

I get you, but I really think it's important to keep this name close to the existing document.styleSheets, since it's doing essentially the exact same thing. If we're still using "styleSheets", we can't get too much shorter than "adopted".

One thing about "css", is that it specifies the exact technology used today for style sheets.

That's not really a big deal here; we're long, long past the point where a new styling language is a particular concern.

We definitely need some array-like structure for the API,

Yup, need basically all the functionality of an Array here. If we could convincingly fake an Array on the platform, that would be something else, but we can't. :/

Rich-Harris commented 5 years ago

I see the need for these operations

As an app developer I would file the following under YAGNI:

I'm also sceptical that I would need to prepend, in practice. I appreciate that there may be some contrived situations in which those operations are necessary, but it doesn't seem to me that the API needs to optimise for them.

I really think it's important to keep this name close to the existing document.styleSheets

I'm not sure I've ever seen anyone use document.styleSheets in the wild. Again, I'm sure it has its uses, but it's not part of most web developers' lived experience, whereas document.adoptedStyleSheets would be. And most usage examples of adoptedStyleSheets I've seen focus on the custom element case, rather than document. So I'm not sure I agree that that's a priority.

Moreover, document.styleSheets is a StyleSheetList rather than a frozen array, so if consistency is a goal then it still falls short.

justinfagnani commented 5 years ago

As an app developer I would file the following under YAGNI:

As someone who maintains code that already uses adoptedStyleSheets I have to disagree.

Managing the list directly is extremely important for app and component-set theming mechanisms and polyfills that might rely on adoptedStyleSheets. Given that multiple actors may be managing the list, being able to tell if the sheets you care about are already in the list, and what precedence they're at is important.

dtruffaut commented 5 years ago

Hum, so you want to be able to test a key, then decide if you insert before of after this key.

harunhasdal commented 4 years ago

If the word adopted is confusing as per multiple parents adopting a child. Maybe .appliedStyleSheets would be a better name.

othermaciej commented 4 years ago

If the acronym is really considered regrettable, .extraStyleSheets is another option.

nordzilla commented 4 years ago

Why not simply .constructedStyleSheets?

The name is a bit longer than .adoptedStyleSheets, but it is clear about exactly what this is for: the structure that holds the style sheets that were created using the constructor.

The only thing I dislike about .appliedStyleSheets is that any of the sheets can be disabled, in which case, they are not applicable.

.extraStyleSheets would probably be fine, but it doesn't communicate any inherent restrictions, especially in the case of getting a NotAllowedError when you try to add a non-constructed sheet.

If I were a developer exploring a new feature called Constructable StyleSheets, I think I would find it intuitive that there is a new attribute called .constructedStyleSheets.

calebdwilliams commented 4 years ago

Ostensibly this API will be used for CSS modules as well, so .constructedStyleSheets doesn't quite fit.

calebdwilliams commented 4 years ago

Just putting some thoughts here, the purpose of this API is to share stylesheets across multiple DocumentOrShadowRoots and to assume styles from CSS modules. If we want to keep the -StyleSheets suffix, perhaps something along the lines of sharedStyleSheets.

connectedStyleSheets could also work (the acronym might be confusing), but the language relates to Node.isConnected which is a property on the Node object that illustrates whether or not the node in question is connected to a context object. In this instance the adjective connected is relating to which style sheets are currently connected to the relevant DocumentOrShadowRoot with an understanding that each sheet that can be connected can also be connected elsewhere (where the analogy fails alongside the analogy for adopted).

othermaciej commented 4 years ago

I think the name should describe what the author intends to do with the stylesheets, not how they were made. One could imagine future ways to create detached stylesheets that don't involve a constructor, but which might still be desirable to attach to a document at some point. So we should not lock in how they are made IMO.

I am not sure why adding non-constructed stylesheets is not allowed currently. Is it because all other current ways of making stylesheets already start out attached to a document? In which case the error isn't that it's not constructed, it's that it is already attached.

nordzilla commented 4 years ago

I think it's reasonable to not tie the name to how the sheets are created in case that were to change in the future.

I am just referring to the spec's description of adoptedStyleSheets:

If any entry of adopted has its constructed flag not set, or its constructor document is not equal to this DocumentOrShadowRoot's node document, throw a "NotAllowedError" DOMException.

rniwa commented 4 years ago

Alright, I've tired to summarize the list of suggestions made so far with the corresponding concerns.

Lodin commented 4 years ago

What about something like cssList? Like a classList, but for style sheets.

rniwa commented 4 years ago

What about something like cssList? Like a classList, but for style sheets.

I think that has the same concern mentioned earlier that it sounds not related at all to document.styleSheets or shadowRoot.styleSheets.

Lonniebiz commented 4 years ago

To me the essence of these things are that "each style sheet listed" is in the form of a javascript variable that points directly to the style in memory and NOT some URL that has to be downloaded and cached from a web server.

This makes me think of names like:

By now, isn't just going to stay adoptedStyleSheets anyway? Verbosity!

I'd be cool with styles, styleSheets, or shadowStyleSheets.

morewry commented 4 years ago

So long as it's not misleading as to the relationship between the stylesheets and the document/shadow root in a problematic way, I think it's nice that a web component's shadow root connectedStyleSheets and a web component's custom element connectedCallback would both use "connected."

...despite them being slightly different senses of connected.

othermaciej commented 4 years ago

Those are different senses of the word connected, I think.

calebdwilliams commented 4 years ago

@othermaciej I disagree. isConnected reports if a particular node is attached to some point in a DOM tree. The true analog would be to let stylesheets (constructed or modules) have an isConnected feature as well, let's pretend for a minute they do:

const sheet = new CSSStyleSheet();
someShadowRoot.adoptedStyleSheets = [ sheet ]; // using the old naming convention on purpose here
console.log(sheet.isConnected); // if the feature exists, I would presume this to be true

Therefore we can ask to what is sheet connected? In this instance it is connected to someShadowRoot. Therefore I do think it makes sense to call this API DocumentOrShadowRoot.prototype.connectedStyleSheets and perhaps introduce the CSSStyleSheet.prototype.isConnected (which I can see being useful, but don't have a hard use case for so I won't push it) or even CSSStyleSheet.prototype.connectedTo which could be a NodeList of every tree the sheet is connected to.

rniwa commented 4 years ago

I don’t think the analogy with node’s connectedness quite works because a single node can be associated with at most one document whereas a style sheet can be associated with multiple shadow trees / documents; this is actually problematic though... We probably want to limit it to the same document somehow. Otherwise we’d end up with global object leaks.

calebdwilliams commented 4 years ago

@rniwa, That makes sense, but unless I've missed something (which is possible, you are certainly better prepared to make that sort of statement than I am), there's nothing about connected that necessarily implies a one-to-one relationship; a Node instance, however, does have that implication, but a CSSStyleSheet and a Node are different beasts, but both can feasibly be connected to some context. That's all I'll say about it, thanks for all the hard work on making this API work for everyone.

zcorpan commented 4 years ago

@rniwa the suggestion in https://github.com/WICG/construct-stylesheets/issues/97#issuecomment-521299640 was not in your list: sheets.

Though I think the concerns against moreStyleSheets and extraStyleSheets also apply to sheets, but, if a short name is deemed more important, this one seems better than css.

There is some precedent: https://drafts.csswg.org/cssom/#the-linkstyle-interface

calebdwilliams commented 4 years ago

@zcorpan That actually makes a lot of sense. If you do let style = document.createElement('style'), the style object will have a property called sheet that is the CSSStyleSheet (or StyleSheet in Firefox) object that would be created here. Calling the API sheets literally makes it a list of sheet objects.

rniwa commented 4 years ago

@rniwa, That makes sense, but unless I've missed something (which is possible, you are certainly better prepared to make that sort of statement than I am), there's nothing about connected that necessarily implies a one-to-one relationship; a Node instance, however, does have that implication, but a CSSStyleSheet and a Node are different beasts, but both can feasibly be connected to some context.

Even if we took that argument, there is another issue with connectedStyleSheets, which is that a ShadowRoot isn't necessarily always connected to a document yet you can still add a stylesheet to such a disconnected ShadowRoot.

That's all I'll say about it, thanks for all the hard work on making this API work for everyone.

pygy commented 4 years ago

What about a name that evokes their position in the cascade?

93 currently says that they take precedence in the cascade, their name could reflect it. If at some point another set with the lowest possible priority is added, it could take a name based on the same logic. firstStyleSheets / lastStyleSheets, or any pair of respective synonyms. early/late, start/end, before/after, etc...

styleSheetsPrologue and styleSheetsEpilogue also fit and add the nuance that they are there in addition to the document.styleSheets which form, metaphorically, the "main body" of the style story.

Edit: minor usability concern, epilogueStyles or epilogueStyleSheets give nicer tab completions (ep<tab>, vs styleSheetsEpilogue clashes with styleSheetSets), but prologueStyles needs four keystrokes to differentiate it from propertyIsEnumerable.

othermaciej commented 4 years ago

Of the various names suggested, I think sheets and css are not good. It would suggest it's all of the Document's or ShadowRoot's stylesheets, but it's not; only the ones attached in ways other than processing of <style> or <link rel=stylesheet> elements.

Furthermore, the DocumentOrShadowRoot interface has a styleSheets IDL attribute, which contains the stylesheets attached in these other ways.

To be a good name, the new property for sheets added programmatically must at minimum be clear on the distinction between it and the styleSheets property. Incidentally, per the current Constructible Stylesheets spec, adopted stylesheets are included in the base styleSheets property, via a monkeypatch of CSSOM.

With all that in mind, here's some potential names:

  1. If their nature as additional is what's important: extraStyleSheets, moreStyleSheets, additionalStyleSheets or short versions extraSheets, moreSheets. But these names would suggest sheets that are in addition to styleSheets, when they are actually a subset.
  2. If the fact that they are added dynamically is what's important: attached, connected, dynamic, added before StyleSheets or Sheets.
  3. If their priority order is what's important, override, highPriority, lowPriority, again with the same suffixes.

After reviewing the specs, I like the names from group 2 the best, as the direct addition/removal of these stylesheets without appearing in the DOM is the thing that's most different about them from other stylesheets. And these names correctly suggest that they are a subset of styleSheets, not an addition.

pygy commented 4 years ago

@othermaciej in literature, the prologue and epilogue are part of the story, but usually with a different pace and/or mood than the main body of text. That doesn't clash semantically with having them readable in the styleSheets list, or having them being StyleSheets...

Also, when discussing them, their dynamic nature will be impllied (they can't be created in any other way), but being able to unambiguously and quickly tell their priority is important.

Also, assuming a "before" variant is added later on, it would end up with an awkward addedBeforeStyleSheets, or whatever derivative of a "dynamic"-based name we would end up choosing...

emilio commented 4 years ago

To be a good name, the new property for sheets added programmatically must at minimum be clear on the distinction between it and the styleSheets property. Incidentally, per the current Constructible Stylesheets spec, adopted stylesheets are included in the base styleSheets property, via a monkeypatch of CSSOM.

Hmm... That's not how Chrome's implementation behaves, fwiw. @rakina / @tabatkins, is that intentional?

emilio commented 4 years ago

(seems a bit weird to me to mix up the two lists of stylesheets like that, fwiw)

othermaciej commented 4 years ago

If we make a successor to adoptedStyleSheets under a new name with better semantics (readonly property to a mutable container, instead of assignable property that freezes anything assigned to it), it doesn't have to inherit the spec language for adoptedStyleSheets. Especially if that language was never actually implemented.

I don't really have an opinion on which is better, except that it should be specified, and ideally tested by WPT tests.

othermaciej commented 4 years ago

It's also possible that I'm misreading the CSSOM or Constructible Style Sheets specs. There's a remote chance that the authors of the spec did not realize that adding to "document or shadow root CSS style sheets" causes the sheets to eb exposed in the styleSheets property. Or it might be very intentional, but the Chrome implementors overlooked this subtlety. Would appreciate clarity from the spec authors. Inclusion in styleSheets or not suggests different choices for naming.

pygy commented 4 years ago

Maybe we can have it both ways? dynamicCSSEpilogue

Throwing other shades of the literary metaphor: foreword/afterword (germanic roots, rather than french/greek), or preamble/postamble (latin, though the latter is a recent addition to the English language).

So, dynamicCSSAfterword? Or dynamicAfterwordCSS, which is more readable in CamelCase IMO.

edit introduction/conclusion also fits.

calebdwilliams commented 4 years ago

@rniwa, According to the spec

An element is connected if its shadow-including root is a document.

Since that definition is specifically about an element, couldn't a CSSStyleSheet's definition be something like: "A stylesheet is connected if it is referenced by one or many DocumentOrShadowRoots?"

rniwa commented 4 years ago

@rniwa, According to the spec

An element is connected if its shadow-including root is a document.

Since that definition is specifically about an element, couldn't a CSSStyleSheet's definition be something like: "A stylesheet is connected if it is referenced by one or many DocumentOrShadowRoots?"

Having a slightly different semantics for connectedness between nodes and style sheets would be too confusing.

blikblum commented 4 years ago

How about localStyleSheets in contrast with the "global" document.styleSheets?

It does not imply the connect state or if is shared or not. Just the scope/locality of the style sheets

calebdwilliams commented 4 years ago

The only real-world analogy for this feature I can think of is when two atoms share an electron and that's just bond. So bound or bonded might work with the suffixes @othermaciej mentioned.

rakina commented 4 years ago

It's also possible that I'm misreading the CSSOM or Constructible Style Sheets specs. There's a remote chance that the authors of the spec did not realize that adding to "document or shadow root CSS style sheets" causes the sheets to eb exposed in the styleSheets property. Or it might be very intentional, but the Chrome implementors overlooked this subtlety. Would appreciate clarity from the spec authors. Inclusion in styleSheets or not suggests different choices for naming.

Oops yeah I think we never intended the adopted stylesheets to be included in the styleSheets list (hence why we named it moreStyleSheets before we changed it). We don't want to reflect them twice (and only one of the list they appear in being mutable). I think names from category 1 or 2 from @othermaciej's post would work fine. https://github.com/WICG/construct-stylesheets/issues/97#issuecomment-581186920

othermaciej commented 4 years ago

Oops yeah I think we never intended the adopted stylesheets to be included in the styleSheets list

OK, filed https://github.com/WICG/construct-stylesheets/issues/118

While filing I also noticed a more serious problem with the spec, which is that the spec does not appear to require adopted stylesheets to be used for styling, since that is defined (I think) by CSS Cascading & Inheritence, not by CSSOM.

calebdwilliams commented 4 years ago

I like localStyleSheets, but the contrast with global styleSheets fails on document. Also, each shadow root does have its own styleSheets property. That might not be a big deal though.

othermaciej commented 4 years ago

local doesn't really explain the difference with the styleSheets property. Whether on document or shadow root, they are not more local than what is in styleSheets.

pygy commented 4 years ago

re. dynamic, specifically, isn't it misleading, given that they are less modifiable than the standard ones?

More generally, regarding the second category in Maciej list, <style> and <link> can also be added. The difference with that the "adopted" ones can't be statically defined. Not sure where it leads us, but it may be food for thought.

Regarding the priority options, the cascade is something that is often ignored by frontend devs that are primarily coders rather than designers, often to their detriment. Having a salient reminder of the existence cascade in the API name may be a good thing.

Edit: rootStyles? They are directly tied to either the document root, or to the shadowRoot, unlike the others that belong to dedicated elements.

othermaciej commented 4 years ago

Good point about dynamic, others can also be added, if a bit more indirectly. I don't see why they are less modifiable though? They are more conveniently modifiable.

The other kinds of styles also apply at the root, even if they originate from a more specific element, so I don't think rootStyles is a clear distinction.

pygy commented 4 years ago

They are more conveniently modifiable.

Oh, indeed, not sure why I had erroneously inferred that the new methods were the only one that applied.

Naming is both a logical and a UX problem, which turns into bikeshedding contests because programmers are often not trained in the latter. There's a method to UX, and there are objective properties that make a good name beyond pure semantics.

Keep in mind that we're toolsmiths here, and that the names are the mental handles for those tools.

They should be:

They should also be descriptive, but I think that the description emphasis should be put on the properties that are important for everyday use, not for first time use.

That's why I don't like the adjectives. They are vague and ethereal, so they often don't stand on their own. You need to tack on "StyleSheet" for disambiguation. "Put the styles in the extraStyleSheets" vs "... in the epilogue".

Using a unique noun roots the the concept in the mental map without redundancy.

With all that said, stylesAddendum has many good properties.

stylesSecretStash also comes to mind.

It is a bit less descriptive (emphasizing the fact that the are out of the DOM tree), but more playful (another important dimension as far as UX goes).

This may not be the best name though... The accronym can be easily misused (trigger warning).You could "put the CSS in the SSS", which is clearly better than the current acronym... It would only be one typo away given the QWERTY layout tough. Cue in harassment lawsuits for accidental typos (or actual harassment disguised as oversight). Edit: this took a dark turn, sorry...

Edit: That general direction may be worth exploring. stylesHideout could work, for example.