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

Add a constructor that takes a Response #49

Closed
tabatkins opened this issue Oct 23, 2018 · 2 comments
Closed

Add a constructor that takes a Response #49

tabatkins opened this issue Oct 23, 2018 · 2 comments
Labels
enhancement New feature or request

Comments

@tabatkins
Copy link
Contributor

/cc @domenic @rniwa

Ryosuke suggested adding a constructor that just takes a URL, so you can construct a stylesheet as easily as adding a link, like document.loadCSSStyleSheet(...).

I thought that Fetch has a lot of options beyond just the URL, and I didn't want to duplicate those in the CSSStyleSheet() constructor (and keep it consistent as Fetch grows and changes). You can just do fetch(...).then(r=>document.CSSStyleSheet(r.text)) pretty easily.

Domenic pointed to WASM as an example where they have a constructor directly take a Response object, so you can build something from a fetch() response even more easily: document.CSSStyleSheet(fetch(...)).

This sounds good to me.

@rakina rakina added the enhancement New feature or request label Dec 10, 2018
@calebdwilliams
Copy link

Is this technically necessary if CSS Modules gets implemented? That seems like a more intuitive interaction model. Besides, if I'm not mistaken, the right way to use the spec as currently implemented in Chrome would be

const sheet = new CSSStyleSheet();

fetch('url').then(res => sheet.replace(res.text));

someShadowRoot.adoptedStyleSheets = [ sheet ];

Which has the added feature of not blocking the use of the sheet while the resource is being fetched and parsed.

@tabatkins
Copy link
Contributor Author

Ah, this was written back when constructing a stylesheet was always an async operation, so the fact that taking a response was async as well wasn't a problem.

Now that we're using a real constructor, and thus produce the initial sheet sync, I don't think this makes sense. It would just very slightly shorten the code you produced:

// instead of
fetch('url').then(res => sheet.replace(res.text));
// you could write
sheet.replace(fetch('url'));

And like, that's nice and all, but it's so minor as to probably not be worth it.

I'll go ahead and close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants