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

Consider a different name for 'adoptedStyleSheets' #97

Closed
Rich-Harris opened this issue Aug 14, 2019 · 61 comments
Closed

Consider a different name for 'adoptedStyleSheets' #97

Rich-Harris opened this issue Aug 14, 2019 · 61 comments

Comments

@Rich-Harris
Copy link

Rich-Harris commented Aug 14, 2019

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
Copy link

dtruffaut commented Aug 14, 2019

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
Copy link

'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
Copy link
Author

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
Copy link

zcorpan commented Aug 14, 2019

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
Copy link

calebdwilliams commented Aug 14, 2019

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.

@travisleithead travisleithead changed the title Consider a different name Consider a different name for 'adoptedStyleSheets' Aug 14, 2019
@Lonniebiz
Copy link

Lonniebiz commented Aug 14, 2019

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
Copy link

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

  • Append one or more sheets to the end of the list
  • Prepend one or more sheets to the start of the list
  • Get length
  • Insert one or more sheets at arbitrary indices
  • Access by index
  • Iterate through the list
  • Remove from arbitrary indices
  • Replace the entire list

@calebdwilliams
Copy link

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
Copy link
Contributor

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
Copy link
Author

I see the need for these operations

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

  • Get length
  • Insert one or more sheets at arbitrary indices
  • Access by index
  • Iterate through the list
  • Remove from arbitrary indices

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
Copy link

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
Copy link

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

@harunhasdal
Copy link

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

@othermaciej
Copy link

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

@nordzilla
Copy link
Contributor

nordzilla commented Jan 24, 2020

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
Copy link

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

@calebdwilliams
Copy link

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
Copy link

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
Copy link
Contributor

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
Copy link

rniwa commented Jan 25, 2020

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

  • css
    Concerns: Confusing with CSS. Does not sound related to document.styleSheets.
  • moreStyleSheets
  • extraStyleSheets
    Concerns: It doesn't communicate any inherent restrictions, especially in the case of getting a NotAllowedError when you try to add a non-constructed sheet.
  • appliedStyleSheets
    Concerns: any of the sheets can be disabled, in which case, they are not applicable.
  • constructedStyleSheets
    Concerns: Naming how they were created is strange; we may want to allow CSS moduels or StyleSheet from style element in the future.
  • connectedStyleSheets
  • sharedStyleSheets
    Concerns: These style sheets are not always shared.

@Lodin
Copy link

Lodin commented Jan 25, 2020

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

@rniwa
Copy link

rniwa commented Jan 25, 2020

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
Copy link

Lonniebiz commented Jan 25, 2020

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:

  • referencedStyleSheets
  • linkedStyleSheets
  • styleSheetPointers
  • styleSheetReferences
  • styleReferenceList

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

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

@morewry
Copy link

morewry commented Jan 25, 2020

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
Copy link

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

@calebdwilliams
Copy link

@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
Copy link

rniwa commented Jan 27, 2020

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.

@emilio
Copy link

emilio commented Feb 3, 2020

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
Copy link

emilio commented Feb 3, 2020

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

@othermaciej
Copy link

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
Copy link

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
Copy link

pygy commented Feb 3, 2020

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
Copy link

@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
Copy link

rniwa commented Feb 3, 2020

@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
Copy link

blikblum commented Feb 3, 2020

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
Copy link

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
Copy link
Member

rakina commented Feb 4, 2020

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. #97 (comment)

@othermaciej
Copy link

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

OK, filed #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
Copy link

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
Copy link

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
Copy link

pygy commented Feb 6, 2020

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
Copy link

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
Copy link

pygy commented Feb 7, 2020

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:

  • unique for unambiguous denomination
  • ergonomic for everyday use including
    • discussing them and writing about them (for both documentation and collaboration)
    • typing them (also having auto-completion in mind, although this may be a secondary concern)
  • sufficiently striking to make first timers realize that it is something they don't know about

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.

  • It ticks all the boxes above ("I'll put the styles in that root's addendum" and "The style sheets in the addendum have the highest priority in the root's cascade" stand on their own without redundant language).
  • It clearly states that such styles couldn't have been part of the document when "published" i.e. sent by the server.
  • It implies a high precedence (because in the press, information in the addendum supersedes the content of the article).
  • so it covers all three categories singled out by Maciej, and then some.
  • It is forwards compatible as far as naming goes. It leaves the door open for a lowest prioritystylesForeword that has (most of) the same 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.

@Lonniebiz
Copy link

Lonniebiz commented Feb 7, 2020

Ha, along those lines: stylePile. Making it rhyme is a must.

@calebdwilliams
Copy link

Another option for a prefix could be root since these sheets are applied to roots rather than only documents:

  • rootStyles
  • rootStyleSheets
  • rootSheets

@Lonniebiz
Copy link

styles - Source

Perhaps comparing adoptedStyleSheets to what is above will also help in its renaming.

What do you call these dom instantiated style sheets? scriptedStyleSheets? They're called constructed style sheets here: https://wicg.github.io/construct-stylesheets/#modifying-constructed-stylesheets

@blikblum
Copy link

assignedStyleSheets ?

@trusktr
Copy link

trusktr commented May 7, 2020

How about something like assignedStyles or localStyles (using "styles" instead of "stylesheets" in the name).

@bkardell
Copy link

bkardell commented Feb 9, 2024

Given this is shipping everywhere it feels like we should consider this issue resolved and close it

@tabatkins
Copy link
Contributor

Yup, we're pretty far past the point where this could be changed now.

@Rich-Harris
Copy link
Author

It's pretty frustrating that an issue with a preponderance of upvotes is closed after almost 4 years of inactivity because 'oops, too late to do anything about it now'. Of course it's too late to do anything about it now, that's why I raised it in 2019!

Things like this are why I and many others don't really consider it worthwhile to engage in the standards process.

@tabatkins
Copy link
Contributor

There were many, many suggestions in this thread, but none of them were sufficiently compelling to change the spec to. Note that I engaged in the thread back when you first posted it, and several additional times after that; the implication that this thread was simply ignored and then closed as a fait accompli is not appreciated.

An important aspect of engaging in the standards process is realizing that not every good idea will result in change.

@Rich-Harris
Copy link
Author

none of them were sufficiently compelling to change the spec to

As decided by whom?

I engaged in the thread back when you first posted it

Where? You responded on Aug 14 2019, the day I opened the issue, and then again two hours ago to close the issue. I don't see any evidence that you responded to the many perfectly reasonable suggestions in between. If GitHub's UI is somehow hiding additional responses, then I apologise.

I'm not stomping my feet saying that every time someone opens an issue it should result in a change. I am saying that when under-designed features get YOLOed into the platform, it reinforces the belief that implementers wield all the power in these discussions.

@tabatkins
Copy link
Contributor

As decided by whom?

Me, the editor. (And no implementor felt sufficiently passionate about it to press for any other name.)

I don't see any evidence that you responded to the many perfectly reasonable suggestions in between.

Ah, you're right. I literally just re-skimmed the thread before posting that, and I think just got a little muddled with some of the other commenters.

it reinforces the belief that implementers wield all the power in these discussions.

That is a correct belief, tho. Standards are a means to coordinate implementations to go in the same direction. The goal is implementations agreeing with each other, but that's entirely voluntary on the implementation's part. Everyone who is not an implementor gives input to the process, arguing their position to attempt to convince the editors and implementations to do something in a particular way, but they have no ability to cause change.

I am saying that when under-designed features get YOLOed into the platform

And now you're continuing to be offensive, so I'm locking this thread.

@WICG WICG locked and limited conversation to collaborators Feb 9, 2024
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