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

Document state should determine whether the CSS has been added #554

Closed
nsaunders opened this issue May 3, 2017 · 5 comments
Closed

Document state should determine whether the CSS has been added #554

nsaunders opened this issue May 3, 2017 · 5 comments

Comments

@nsaunders
Copy link

Svelte components add their stylesheets to document.head once per component type. Whether the stylesheet has already been added is currently determined by a private variable added_css inside the component module. While in most cases, this flag should be accurate, this is not always the case in the REPL (see svelte.technology#93) and possibly other environments as well. I believe it is more correct to attach some kind of identifier to the style element and then query the document.head element to determine whether it already exists. In my opinion, the accuracy of this approach is well worth the performance hit.

@Conduitry
Copy link
Member

I think this seems like a reasonable enough change. I've pushed a branch but haven't opened a PR yet.

@Rich-Harris
Copy link
Member

Agree with this in principle. Would using an id instead of an attribute selector minimise the performance hit? I could be wrong but IIRC browsers are better at querySelector('#thing') or getElementById('thing') than querySelector('[thing]')

@nsaunders
Copy link
Author

@Rich-Harris
Copy link
Member

Thanks. I suppose the real comparison is this one: https://jsperf.com/getelementbyid-vs-qs-id-vs-qs-attr

Seems conclusive!

screen shot 2017-05-03 at 2 18 36 pm

screen shot 2017-05-03 at 2 19 39 pm

@nsaunders
Copy link
Author

Good find. Thanks guys for your consideration! I am looking forward to a perfect REPL experience once this is resolved. :-)

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

No branches or pull requests

3 participants