Switch back to Real Constructors™

[tabatkins]

We switched away from constructors to instead use factory functions for two reasons:

1. Some of the loading details get easier if it's clear what document the stylesheet is associated with. While you can find an appropriate document to associate with a normal constructor (I forget what the right term is, but it's used elsewhere), it's a little weird. A factory function on document makes this obvious.

2. We want at least some of the constructors to be async, and async constructors aren't going to be a thing.

---

@plinss suggested that we can get around both of those issues:

1. If we really need to associate it with a document, we can just require passing that as an argument to the constructor. In new CSSStyleSheet(document) versus document.createCSSStyleSheet(), the former is even slightly shorter!

2. The constructor can take some text (or whatever, there's a few forms), and create an inert stylesheet. You can then activate it with either .parse() (which is sync, and which throws if there's an @import) or .load() (which is async and allows @import). If we add a Response-taking constructor like in Add a constructor that takes a Response #49, that particular stylesheet can only be .load()ed; .parse() would throw.

---

Thoughts? @bzbarsky @annevk @dbaron @domenic TAG people?

[domenic]

I like point 1. Point 2 I am less fond of, because it adds this new "inert style sheet" concept and a mini state machine, and causes what I would assume is the common case to be longer. (And it seems easy to forget, leading to confusion.) Another way of stating it, is that it breaks the invariant that at any given time a CSSStyleSheet is "usable". E.g. you have to answer questions about what happens if inert style sheets are put in adoptedStyleSheets or given to customElements.define or similar.

I'll open a separate issue as to whether we need the document at all.

Here's a third alternative (in addition to the factory-function model and the OP's model): a text-accepting constructor, and static factory functions for import-containing strings or responses. However I'm not sold on this because the constructor hasn't really de-magicked anything; the factory functions still use some magic construction ability.

The OP's model is quite nice from a de-magicking perspective though. Which is good. Maybe we should do it?

[tabatkins]

E.g. you have to answer questions about what happens if inert style sheets are put in adoptedStyleSheets or given to customElements.define or similar.

My preference is that it would just be invalid, throwing or whatever.

You're right that what's really being constructed is an InertStyleSheet, and then .parse() returns a CSSStyleSheet, .load() returns a Promise<CSSStyleSheet>, etc.

[bzbarsky]

We need a document for @import fetches. That said new CSSStyleSheet has an obvious document: the one for the Window the constructor comes from (the constructor's Realm). So there's no need to pass it in explicitly. I don't think doing this is particularly weird; this is how XMLHttpRequest, WebSocket, Worker, etc, etc determine the window/document they are associated with.

In addition to @domenic's questions about where you can insert an inert stylesheet, can one use CSSOM (or parts of it, which parts?) on inert sheets? If there's a separate InertStyleSheet type that answers that question, of course. But if we do that, then why is that better than factory functions? Seems like more complexity but fundamentally the same...

// cc @emilio

[domenic]

You're right that what's really being constructed is an InertStyleSheet, and then .parse() returns a CSSStyleSheet

This is a really interesting framing. It raises the question: should InertStyleSheet be a first-class object? Does it add any value over a string?

My tentative answer is negative; it seems like it does nothing more than a string, except that it gives us a place to hang methods. In which case inverting it and using factory functions is probably simpler.

This exploration has helped me understand the "de-magicking" aspect a lot better though:

• In the InertStyleSheet version, parse() does magic to create a CSSStyleSheet
• In the createStyleSheet version, the factory function does magic to create a CSSStyleSheet
• In the two-state CSSStyleSheet version, parse() does magic to make the CSSStyleSheet "active".

There's a conservation of magic here, and I (and others) have been very focused on avoiding the "creating objects without constructors" magic. Showing that you can transform that into "changing internal state" magic in an isomorphic way makes me reconsider my stance on how bad non-constructible classes are...

[plinss]

My suggestion wasn't to create an InertStyleSheet, but rather to simply create a proper CSSStyleSheet via a constructor. Possibly passing the document to the constructor (if really needed).

The text would be passed to the .parse() or .load() methods (or a URL, or a fetch response).

Why shouldn't you be able to construct a stylesheet, then construct style rules and append them to the sheet, never calling .parse() or .load() at all?

[annevk]

I think the static factory taking a Response and returning a promise is fine. It can be explained in terms of fetch(), a check on the MIME type, response.text(), and running the text-based constructor.

[tabatkins]

So to be more specific, new CSSStyleSheet() would specifically create a working, empty stylesheet. You can immediately use this, append rules to it, etc.

Then .parse(DOMString) would take a string, synchronously parse it, then wipe the contents and replace it with the newly-parsed stuff.

.load(DOMString) would asynchronously parse the string (and fetch imports, etc), then queue a task to wipe the contents and replace it with the newly-parsed stuff, then fulfill the promise with the stylesheet.

.load(Response) would work similarly.

This structure sounds a lot better to me, and makes me think the constructor approach is a reasonable way to go.

[calebdwilliams]

As a user I kind of like the idea of inert style sheet, I’ve even brought up the idea before.

Ultimately I don’t think there’s really a difference between a constructed style sheet and an inert one other than a style sheet marked as inert isn’t immediate added to document.styleSheets. It seems like any style sheet that is constructed should just be a CSSStyleSheet object that is inert until adopted. Then as both I and now others have said, adding a parse method for asynchronously applying rules just makes sense. That way constructed sheets are immediately usable, but rule addition and parsing could happen at some later time.

[bzbarsky]

So if new CSSStyleSheet() creates a fully working sheet, we're back to the question of what happens when @import rules are added after the sheet is attached to multiple documents.

[rakina]

So if new CSSStyleSheet() creates a fully working sheet, we're back to the question of what happens when @import rules are added after the sheet is attached to multiple documents.

I think we can just check whether the stylesheet has been used in a document different than the associated one, and then just throw if load that has import rules is called on it?

[bzbarsky]

I was actually talking about the insertRule()case, but I suppose we could do the same thing in that case.

It would involve tracking this state on the sheet explicitly, which is a little annoying but should be doable.

[tabatkins]

Argh, I didn't even realize you could add @import rules programmatically. They don't even need to be first in order; the browser will automatically move them to the front when serializing (at least, Chrome does). testcase

So yeah, tracking a bit on constructed stylesheets that just says "no inserting @import rules" (and any future directly-fetch-causing rules that we might add) is probably the best way to handle things. The only way to add an @import is to do one of the async loads.

It's not like @import is particularly necessary; if you're doing programmatic stuff, you can just fetch() the sheet and construct another one.

[emilio]

Note that your test-case works because insertRule without an index effectively prepends, not appends. You can't insert @import anywhere, that would throw a HierarchyRequestError.

[bzbarsky]

Argh, I didn't even realize you could add @import rules programmatically.

I thought that allowing that was the main upshot of the thing @plinnss asked for in the meeting...

They don't even need to be first in order

They do, actually.

[tabatkins]

Note that your test-case works because insertRule without an index effectively prepends, not appends.

Ahahaha, kk. Cool then, that at least makes more sense.

I thought that allowing that was the main upshot of the thing @plinnss asked for in the meeting...

No, afaik (@plinss can correct) this was irrelevant to him. He just wanted Real Constructors™ if possible, and that involves making Real Stylesheets™, and a side-consequence of that is that you can insert rules into them immediately.

[plinss]

Correct, my primary motivation is getting a real constructor and having stylesheets behave like regular JS objects as much as possible. The constructor should create a fully usable stylesheet that can be loaded or simply used as-is and manipulated by it's normal API.

Of course I'd like the stylesheet to behave rationally for all rule insertions, but that's another discussion.

[rakina]

So do we want to disallow inserting @import rules in constructed stylesheets in all cases, or do we want to disallow it if it's already used in a different document? The first one is stronger but easier to track because we just have one bit on every constructed stylesheet like @tabatkins said.

[rakina]

Oh, nevermind. We should disallow inserting @import in general according to #56, but we'd disallow @import on load()s it if it's already used in a different document. We'd need to track where we use the stylesheets (which we're already doing anyways since we want to send updates happening to the stylesheets to each DocumentOrShadowRoot it's used on).

One thing I'm not really clear on is a detail on load(DOMString) from @tabatkins' comment above. The function will be called on the CSSStyleSheet, so that stylesheet will eventually get its new content from the string, but the function also returns a Promise<CSSStyleSheet>, where the value after resolving will be that same CSSStyleSheet?

[domenic]

.load(DOMString) would asynchronously parse the string (and fetch imports, etc), then queue a task to wipe the contents and replace it with the newly-parsed stuff, then fulfill the promise with the stylesheet.

One thing I'm not clear on is what is the proposal for the contents of the stylesheet between the time when .load() is called and when it fulfills. Does it keep the old rules? Can you modify/insert rules before loading time?

Maybe it's immediately replaced with the rules derived from the new text, and the promise just signals when imports complete? But then I'm unclear why .parse() exists... it seems strictly less powerful.

(Side note: we'll probably need some new names. .parse() does not communicate "replace all my rules with ones derived from this text", and .load() does not communicate "replace all my rules with ones derived from this text, allowing imports and with a promise for when those finish". But, we can figure out names later, after semantics.)

[tabatkins]

but the function also returns a Promise, where the value after resolving will be that same CSSStyleSheet?

Yes, that way you have convenient immediate access to the stylesheet in the resolution function, without having to stash it elsewhere.

One thing I'm not clear on is what is the proposal for the contents of the stylesheet between the time when .load() is called and when it fulfills. Does it keep the old rules? Can you modify/insert rules before loading time?

Intention is that you can modify/insert rules before loading time, but they're wiped out when .load() fulfills. (We'd queue a task to actually replace the contents of the stylesheet and fulfill the promise, so the timing isn't problematic.)

But then I'm unclear why .parse() exists... it seems strictly less powerful.

It's sync; as soon as it returns, the stylesheet already contains all the rules you wanted to insert. That's the sole advantage of it.

Side note: we'll probably need some new names.

Good point. I don't want to make them too long, but maybe .replace() and .replaceSync()?

[domenic]

Intention is that you can modify/insert rules before loading time, but they're wiped out when .load() fulfills.

That seems really weird and frustrating. It puts you in this intermediate state where anything you do is about to get wiped out.

Why can't it immediately insert the rules?

[bzbarsky]

Intention is that you can modify/insert rules before loading time, but they're wiped out when .load() fulfills.

UAs currently do not expose the rule lists of sheets that are being loaded, which enables streaming parsing, parallelized parsing, and the like. Forcing parsing into some temporary followed by a synchronization point doesn't seem like a good idea to me...

I would strongly prefer that calling load() puts the sheet into a state where you can't inspect or modify its rule list until the load completes.

[bzbarsky]

Why can't it immediately insert the rules?

That forces the parse to block the main thread, right? For large sheets this can be quite noticeable. We saw significant responsiveness improvements on some pages when we moved parsing for sheets from the network off the main thread....

[tabatkins]

@domenic I'm a little confused - you were present in the earlier discussions about sync vs async parsing, so I don't understand what points you're trying to make now. Are you trying to say that we should add a sync fetch to the platform?

[bzbarsky]

I think @domenic wants to parse sync, insert the rules sync, but load the imported sheets async.

Again, I would much rather this were all an async process.

[domenic]

@tabatkins no, I am proposing that we add the rules immediately, and then resolve the promise after all the import fetches complete. The same as if you use addRule today: the fetch happens async, the rule gets added sync.

@bzbarsky see #38 for why we have a sync method of adding rules, even if that can be costly. (createCSSStyleSheetSync() in the current spec, styleSheet.parse() in @tabatkins's proposed revision.) But, I take your point that we may also want an async version for large sheets.

[bzbarsky]

I understand why we have a sync thing. I'd just prefer that we minimize the amount of sync work the async thing (which we also have) does.

[domenic]

Got it. OK, so abandon my direction. We can re-focus on the question of what is the state of the stylesheet while the .load() promise is pending. (And/or the question as to whether this new design is really better than the original one.)

[tabatkins]

The intention of "let people insert rules, but if they call load()/parse(), blank out the sheet when they complete" was to avoid the "inert stylesheet" state. The constructor should, if at all possible, return a workable, usable version of the object, and an empty stylesheet is totally usable. There's no particularly good reason to make the sheet unusable at this point.

Doing a full replace in the async case makes things simpler and more predictable; it answers the question of what to do if you call .load() twice, for example. In the sync case, eh, whatever, I was just trying to be consistent.

I would strongly prefer that calling load() puts the sheet into a state where you can't inspect or modify its rule list until the load completes.

This seems reasonable to me. I wanted to avoid the sheet starting in an unusable state, but having the .load() call force it into an unusable state until it finishes seems reasonable and useful; it avoids footguns.

@rakina, what do you think about that? Upon calling .load(), we synchronously set the rules list to null, and track a bit that disallows modifying the rules of the sheet. Then we unset that when the load finishes and the rules list gets set back up again.

[rakina]

@rakina, what do you think about that? Upon calling .load(), we synchronously set the rules list to null, and track a bit that disallows modifying the rules of the sheet. Then we unset that when the load finishes and the rules list gets set back up again.

I think this is OK, though this means if we've already used that stylesheet somewhere, they will lose all those rules, and there'll be a time period where nothing is applied. Since we're gonna overwrite them anyways that's fine, we just have to make it clear to those who want to use this.

[rakina]

Resolved with #61, the spec is updated now.