Jss: Does detaching sheets make sense?

[kof]

We need to prove/disprove that detaching styles speeds up the nodes insertion and selectors for big apps. Thanks to @nmn for pointing this out.

For this we need to create a benchmark that does appr. the same.

[nathanmarks]

I had the same thought which is why I am not detaching sheets in jss-theme-reactor and counting refs.

What do you want to use as a reference point for a "big app"?

[umidbekk]

+1 for disabling detaching. It's completely unnecessary to keep DOM clean from unused style tags (unless you need to support IE)

[kof]

+1 for disabling detaching. It's completely unnecessary to keep DOM clean from unused style tags (unless you need to support IE)

sheet.detach method itself is not a problem, jss is a rendering tool, its fine to have it. A different question is if integrations like react-jss or jss-theme-reactor need to count refs and detach/attach sheets and if yes, for what reasons.

One thing that I can think of is just for the sake of smaller amount of style nodes in the dom.

[kof]

What do you want to use as a reference point for a "big app"?

We can take some big site or app as an example (amazon, ebay) or anything else and try to measure document.createElement() performance. Also it would be good to know if there are selectors which can havily influence this performance.

[kof]

Also I may be wrong in the assumption that we can measure rendering performance by measuring document.createElement + appendChild. Any other ideas?

[nathanmarks]

That works, but why not also make it end-to-end and test using jss? (can be in addition to simple createElement + appendChild tests in order to ensure that there aren't other areas of the abstraction causing issues)

[kof]

@nathanmarks do you want to give it a try?

[nathanmarks]

Sure, this weekend i'll have more time.

[cvle]

Same thought here. The only thing I can think of is that it might make sense for a huge app to clean up unused styles before changing pages.

[iamstarkov]

I think jss bindings, like react-jss should detach/delete sheet's style tag when component is going to be unmounted to keep only relevant style tags on the page

[kof]

its already the case

[kof]

I am questioning the need for this.

[iamstarkov]

i would say we dont change this part. otherwise, one will end up with style tags explosion

[nmn]

There is a tradeoff to be sure. Removing and adding styles often can cause perf problems. But for large apps, having too many styles can also cause problems. There is no ideal solution for apps of all sizes.

[rosenfeld]

When testing components in isolation it's important to verify that all needed stylesheets have been attached properly. If we can't detach those upon unmounting a component for the next test, then a previous test could be attaching the stylesheet which would cause the current test to pass when it shouldn't. Currently detaching is not useful because it doesn't apply any reference counting, so when detaching on unmount it would detach also from other mounted components still using that sheet. Ideally detach/attach should implement reference counting in order to be useful.