Shall we disallow @import in constructed stylesheets?

[TakayoshiKochi]

The original comment from @rniwa:
WICG/webcomponents#468 (comment)

This decision affects many other issues, so we'd like to get this resolved.

[calebdwilliams]

I guess I fail to see how and why this would need to be different than doing the following:

const sheet = document.createElement('style');
sheet.innerHTML = `
  @import "/path/to/style.css";
  * { color: tomato; }
`;
// some action that would actually add the sheet to to a `StyleSheetList`

I think @rniwa's concern was coming up with how things are parsed. Is there any particular reason they would need to be parsed differently than the action above? If the sheet is constructed it seems reasonable to assume that there is an intent to use it so resource fetching could happen the same as if you were to use an HTMLLinkElement or HTMLStyleElement.

[rniwa]

If allow @import, then we probably have to make the constructible stylesshet an event target and fire load & error events as appropriate. We also have to define how & when those imported stylesheets are fetched.

Honestly, it's fine if you wanted to spend defining those things now that we've found a workaround to use WICG/webcomponents#468 without defining constructible stylesheets (use styleElement's sheet). It just means that this feature (constructible stylesheet) would be further delayed because spec'ing all those details would take time.

[calebdwilliams]

I think this makes sense as an EventTarget and would definitely encourage use more. That's just my opinion as someone interested in using this spec, though. I could see that potentially being a later addition to the spec though …

[rniwa]

If we do allow @import, we'd have to answer the following questions:

1. With respect to which URL, is this URL resolved? Especially in the case where this stylesheet is defined in a html module (HTML Modules webcomponents#645).
2. Should it ever affect the timing of load event? For example, if an element which uses a constructible stylesheet has a pending stylesheet, should it prevent load event on document/window?
3. When does the fetching @import start? As soon as the constructor is called? Or when an element which uses this stylesheet is first instantiated?
4. Should this be considered as style-inline for the purpose of CSP style-src directives? If not, what CSP style-src should we use/add?

[TakayoshiKochi]

@rniwa thanks for the list!

For 1, #10
For 2, as this is a kind of deferred construction, I think no.
For 3, that depends on how #10 resolves, I feel like starting fetch when its base URL is determined.
For 4, filed a new issue #26

[TakayoshiKochi]

From internal discussion within Google, we would like to start implementation disallowing (ignoring) @import rules. Once spec edit is done, we would like to close this issue.
But feel free to update this issue, we would like to listen to important use cases where @import is useful.

[bzbarsky]

Have you spoken to the CSSWG? You can't just create you own "CSS" syntax willy-nilly and call it "CSS".

[TakayoshiKochi]

I was not familiar with the formal CSS process, but will definitely talk to.
I'll put an agenda item on CSSWG once the spec edit is done.

[calebdwilliams]

As someone interested in using this feature, I think it would be a mistake to disallow/ignore @import rules. It's foundational to how CSS is used outside of this feature, changing the language because implementation is difficult is a violation of the priority of constituencies.

Without this the feature loses a lot of impact when it comes to properly modularizing constructed stylesheets.

[rniwa]

Note that I'm no longer of the opinion that @import should be disallowed. It was a stop-gap measure to unblock the styling of custom elements but now that we've found a workaround for that, we don't need to rush this feature through. Let's figure out how @import affects the constructible stylesheet.

e.g. we may want to add a static function like StyleSheet.createFromString() which returns a promise to get resolved with a concrete StyleSheet when all @import are resolved & fetched.

[TakayoshiKochi]

Thanks for the feedback, I'm happy to see @rniwa is not against having @import now.

Maybe we can close this as "we have @import", but until we figure out other issues that @import blocked on, let's keep this open.

[TakayoshiKochi]

@rniwa I'd like to merge StyleSheet.createFromString() idea into existing threads #2 or #9.
#2 is more about making it asynchronous even for parsing the stylesheet, and #9 is more focused on knowing when it's ready.

[calebdwilliams]

@rniwa @TakayoshiKochi How would StyleSheet.createFromString different than the constructor? The point of the constructor would be to create a stylesheet from a string, correct? If you just make the constructed object an EventTarget you would simplify the API and reduce potential confusion.

[bzbarsky]

How would StyleSheet.createFromString different than the constructor?

It would return a Promise<StyleSheet>, not a StyleSheet.

[tabatkins]

Note that in my current proposal-sketch for a generic TypedOM-based CSS parser, all the "large" parsing functions (in particular, parseStylesheet()) are Promise-returning, so you don't block on a potentially-large parse.

This applies just the same here, and suggests we should only have a parsing factory function that returns a Promise, and not have a constructor. Is there a good reason we have to allow synchronous creation, particularly sync creation that's easier to type than the preferred async method?

[calebdwilliams]

@bzbarsky, I get that, it just feels redundant. I actually like the idea of only having an async function in general, but worry what that might mean for the default stylesheets proposal unless that's capable of taking a promise object, too (waiting for the stylesheet to resolve before defining a custom element might have some weird side effects):

const defineElement = async () => {
  const style = await StyleSheet.createFromString(`/* blah blah blah */`);
  customElements.define('my-el', class extends HTMLElement { /* stuff */ }, { style });
}

defineElement();

If style took some time to load, would that mean that my-el couldn't be upgraded until style resolved?

Would that be any different than using a constructed object if it were assuming it's an EventTarget?

[TakayoshiKochi]

If style took some time to load, would that mean that my-el couldn't be upgraded until style resolved?

Yes, from what your code looks like.

Would that be any different than using a constructed object assuming it's an EventTarget?

The answer depends on how we define when the promise is considered resolved, i.e. if (1) it resolves when parsing is done or (2) it resolves when all dependent loading is done (which is the timing that load event will fire). I think people here assume (1).

At this moment I think we assume the constructor returns when parsing of the given string is done, and the resulting stylesheet can be fed to a custom element definition, but @imported sheets could be delayed after upgrade and initial painting of the custom element is done. (This also assumes that you pass the CSSStyleSheet reference to custom element default stylesheet, not a copy or a snapshot of it)

[rniwa]

I think we need to resolve the promise when the dependent loads are done. Waiting just for the parsing to be finished will be quite useless in terms of determining when it's ready to use a stylesheet.

[calebdwilliams]

@rniwa, would that necessarily block the definition and potential upgrade of custom elements?

Also, is it your belief that the constructor should disallow @import whereas the static method would not? That was the spirit of my original question.

Would the style attribute on the custom element config object be able to accept one of these Promise<StyleSheet> object?

[rniwa]

I don't think we're gonna allow constructor at all from what I'm hearing since CSSWG doesn't want to use a synchronous API for anything that involves parsing. That makes sense because in theory we could be parsing CSS in a separate thread.

One problem of accepting this stylesheet promise when defining a custom element is it becomes unclear what happens to the upgraded custom elements between the time such a custom element is defined, and it's used. Does it have display:none by default? But that would cause FOC once the stylesheet is loaded? If so, do we need to delay the reddening? What if the browser engine had already rendered the page? etc... I think it's simplest to leave these questions to people writing custom elements, and have them define a custom element only when the stylesheet is ready to use. Other (dependent) custom elements that use such a custom element can wait for that to happen via customElements.whenDefined(~).

[calebdwilliams]

@rniwa Thanks for that explanation. I guess my concern here would be that the developer experience wouldn't be great as far as syntax is concerned unless some tools were built to abstract the definition process. Assume there are 10 different stylesheets that need to be defined for custom elements, that's 10 different async functions.

A follow up question is what happens when an @import fails? Does the rest of the stylesheet still parse? Say, your imported sheet is for CSS custom properties as I've outlined in #24 and the importer has fallbacks set up? For the record, this is my primary interest in @import for constructible stylesheets (composing constructed stylesheets).

[TakayoshiKochi]

@calebdwilliams I am naively assuming that when you have 10+ different stylesheets for each different custom element, you can put those promises in an array and use Promise.all(), and then register those custom elements.

For the latter let's have a separate issue.

[calebdwilliams]

@TakayoshiKochi, @rniwa, @tabatkins
Does it even make sense to think of those sorts of use cases in terms of @import or would it be better to build out a true composition system similar to what I outlined in the first comment of the above issue #24?

[rniwa]

I think that's up for a debate. Someone has to come up with a concrete proposal about how @import may work in a constructible stylesheet.

[rakina]

The draft spec is recently updated with how @import works on the new factory function. Basically the Promise<CSSStyleSheet> won't resolve until the imports finish, and will reject if any import fail (consistent with ES modules).Probably need to add details on the reason for rejection (maybe just say some imports failed loading, or do we need to specify more clearly?).

Anything else we should add?

[rniwa]

I think we can close this bug now.

[rakina]

Conclusion: @import should work, details on #35.