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 adding StyleSheet.createFromString() which returns Promise<StyleSheet> #2

Closed
TakayoshiKochi opened this issue Feb 5, 2018 · 10 comments
Labels
enhancement New feature or request needs resolution Needs consensus/resolution before shipping

Comments

@TakayoshiKochi
Copy link
Member

The original proposal defines the constructor running in a synchronous manner, but can it return Promise<CSSStyleSheet> instead, allowing the implementation to do lazy CSS parsing of the given text?

@TakayoshiKochi
Copy link
Member Author

Firefox does (will?) parse style sheets in different threads, so it would be nice if the main thread won't be blocked by waiting the parsing to finish.

JavaScript constructor is just a function, so it is free to return any type, but as this is an interface usually defined in .idl file for rendering engines, their binding generators have to take care of that. As far as I researched there is no precedent example of a constructor returning a promise (e.g. Asynchronous extended attribute for constructor does not seem to exist), so we may define this as a factory method in some interface, which looks like methods (e.g. json()) in Body (fetch spec).

TakayoshiKochi added a commit to TakayoshiKochi/construct-stylesheets that referenced this issue Feb 6, 2018
All 5 inline issues are migrated to GitHub.

WICG#2
Shall we allow asynchronous style sheet parsing?

WICG#3
moreStyleSheets needs better name

WICG#4
Shall we include added stylesheets in `document.styleSheets`?

WICG#5
StyleSheetList will need to define a constructor, accepting a `sequence<StyleSheet>`.

WICG#18
Define how origin is determined for a constructed stylesheet
@TakayoshiKochi TakayoshiKochi added the enhancement New feature or request label Apr 17, 2018
@TakayoshiKochi TakayoshiKochi changed the title Shall we allow asynchronous style sheet parsing? Consider adding StyleSheet.createFromString() which returns Promise<StyleSheet> May 25, 2018
@TakayoshiKochi TakayoshiKochi added the needs resolution Needs consensus/resolution before shipping label May 25, 2018
@TakayoshiKochi
Copy link
Member Author

Let's repurpose this for StyleSheet.createFromString() factory method for asynchronous parsing, spun off from #25 (comment)

@tabatkins
Copy link
Contributor

IDL contructors, unfortunately, can't return Promises. (I've opened whatwg/webidl#563 just in case.) So yeah, .createFromString() is currently the way to go.

Do we want to disallow synchronous parsing entirely, so the constructor can synchronously produce an empty stylesheet only?

@TakayoshiKochi
Copy link
Member Author

@tabatkins we can only expose .createFromString() and disallow new CSSStyleSheet(). Authors can easily wrap the factory method in a function to synchronously return generated stylesheet.

@tabatkins
Copy link
Contributor

Well, no they can't; there's no way to synchronously await a promise.

@calebdwilliams
Copy link

I think still having a constructor for an empty stylesheet would be advantageous (for certain use cases), especially if there could eventually be an instance method to append rules from a string:

const sheet = new CSSStyleSheet();
sheet.addRulesFromString(` :host { color: tomato; }) `)
  .then(sheet => console.log('success', sheet))
  .catch(console.error);

So the sheet would still be usable before parsing is completed, but would have the same Promise<StyleSheet> return value.

@tabatkins
Copy link
Contributor

That was my thought - allow the constructor to work, but don't accept any content.

The suggestion to append to the stylesheet async makes good sense!

@calebdwilliams
Copy link

calebdwilliams commented Jun 7, 2018

@tabatkins @TakayoshiKochi
Would that kind of feature make sense as an enhancement in this proposal or would that need to be something separate?

@TakayoshiKochi
Copy link
Member Author

TakayoshiKochi commented Jun 8, 2018

I think we can resolve the final entrypoints of this feature as

new CSSStyleSheet()

to create empty CSSStyleSheet synchronously, and

Promise<CSSStyleSheet> CSSStyleSheet.createFromString(string)

to parse and create CSSStyleSheet asynchronously.

The details have to be fleshed out elsewhere, but let's close this if no one objects in a few days.
I'll make edit on the spec.

@annevk
Copy link

annevk commented Jun 20, 2018

Firefox will have to continue support synchronous parsing due to <style>, but maybe we shouldn't encourage that in API form.

aarongable pushed a commit to chromium/chromium that referenced this issue Jul 9, 2018
We previously implemented constructable CSSStyleSheet synchronously,
but discussions in WICG/construct-stylesheets#2
have gravitated away from that and we are now interested in asynchronous
creation of CSSStyleSheet, but also provide synchronous constructor that
only result in an empty sheet.

This CL changed the constructors to not accept CSS text. This also
changes part of the constructor where we process passed MediaList
data to copy the given MediaList instead of using the same instance
to avoid introducing the concept of mutable MediaLists.
See: WICG/construct-stylesheets#13

CL for promise-based API: crrev.com/c/1126754

Bug: 807560
Change-Id: I0aeb052b63e45d81fc46cde3052f3d134afa16fa
Reviewed-on: https://chromium-review.googlesource.com/1126898
Reviewed-by: Fergal Daly <fergal@chromium.org>
Reviewed-by: Hayato Ito <hayato@chromium.org>
Reviewed-by: Takayoshi Kochi <kochi@chromium.org>
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573233}
aarongable pushed a commit to chromium/chromium that referenced this issue Jul 17, 2018
We previously implemented constructable CSSStyleSheet synchronously,
but discussions in  WICG/construct-stylesheets#2
have gravitated away from that and we are now interested in asynchronous
creation of CSSStyleSheet.

This CL added Document.createCSSStyleSheet that returns a
Promise<CSSStyleSheet>, while also removing the exposed CSSStyleSheet
that was previously exposed. Note that because CSS parsing is still
done on the main thread, this function is actually still blocking.

The previously implemented CSSStyleSheet is changed to only produce
empty CSSStyleSheets in this CL: crrev.com/c/1126898

Bug: 807560
Change-Id: I9f9d17ae04829ff399ae384f8b3a6d97a3b0613b
Reviewed-on: https://chromium-review.googlesource.com/1126754
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Hayato Ito <hayato@chromium.org>
Reviewed-by: Takayoshi Kochi <kochi@chromium.org>
Reviewed-by: Fergal Daly <fergal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575600}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs resolution Needs consensus/resolution before shipping
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants