wilsonpage / fastdom

Eliminates layout thrashing by batching DOM measurement and mutation tasks
6.83k stars 240 forks source link

Does changing class require fastdom? #103

Closed oppianmatt closed 7 years ago

oppianmatt commented 7 years ago

If using element.classList.add/remove does that require fastdom mutate? Or only if measuring after changing classes? Or just to be sure should we always add/remove classes in a mutate?

wilsonpage commented 7 years ago

It depends what style properties the classList change adds/removes from the DOM. For example if you add .foo and that class attaches a property that affects layout (eg. display, width or height) then that should be run within a mutate block.

If you're unsure, or for safety, use fastdom.mutate().

On Mon, 3 Apr 2017 04:58 Matthew Jacobi, notifications@github.com wrote:

If using element.classList.add/remove does that require fastdom mutate? Or only if measuring after changing classes? Or just to be sure should we always add/remove classes in a mutate?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/wilsonpage/fastdom/issues/103, or mute the thread https://github.com/notifications/unsubscribe-auth/AA-Sh2jkK4NTuGZTDCotDfB1UYrGxgclks5rsNDCgaJpZM4MxYhD .

oppianmatt commented 7 years ago

I had the same thought as you but then I read http://kellegous.com/j/2013/01/26/layout-performance/

which claims that browsers use an on demand model of layout, and will avoid calculating layout until needed with an example

using fastdom and it's great, but trying to stop a brief flash of content due to a class not being applied fast enough I was hoping if I didn't have to queue the class change in the mutate phase that it would show sooner (and be ready for anything that runs in the measure phase)

wilsonpage commented 7 years ago

You're right, browsers do batch up layout changes until the next render cycle. The real answer is that it all depends. It's perfectly safe to mutate layout triggering style properties so long as no other code requests to measure the DOM in the same sync task.

It depends how much you trust other engineers and third-party code in your app to do avoid this.

On Mon, 3 Apr 2017 10:15 Matthew Jacobi, notifications@github.com wrote:

I had the same thought as you but then I read http://kellegous.com/j/2013/01/26/layout-performance/

which claims that browsers use an on demand model of layout, and will avoid calculating layout until needed with an example

using fastdom and it's great, but trying to stop a brief flash of content due to a class not being applied fast enough I was hoping if I didn't have to queue the class change in the mutate phase that it would show sooner (and be ready for anything that runs in the measure phase)

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/wilsonpage/fastdom/issues/103#issuecomment-291192712, or mute the thread https://github.com/notifications/unsubscribe-auth/AA-Sh2sM4cZauNnyY9pB3zs4z5r_fmDqks5rsRsKgaJpZM4MxYhD .

paulirish commented 7 years ago

Yeah I would recommend any changes to classList are done within a mutate block. I know Blink has some smarts where a class without any style attached will not be treated as an invalidation (e.g. document.body.classList.add('js-loaded')).. however it seems tenuous to assume that will be handled the smart way by all browsers.

So yeah. In my mind, fastdom exists because trust is hard at scale, so you should def wrap all those classList mutations in mutate() :)


i am a little curious about this FOUC situation though.. @oppianmatt in this particular example are there any other measure or mutate changes in this frame of work? Also, is this happening in an event handler (which?) or just when the script is evaluated?

oppianmatt commented 7 years ago

Ok so putting the class change in the mutate is never a wrong move.

What about reading the class list? That's safe and doesn't need measure?

It's not a fouc on load. It's a match height plug in I'm writing using fastdom. One feature is hiding rows and it sets a class. It's slightly quicker without having to wait for mutate. I set the class and then measure. With mutate I have to set the class in mutate phase, and use a promise to tell me when it's done and then queue up measure stuff which might remove the class at the end which waits for another mutate. So the class is seen for longer. Hence a 'flash' of content.

On Mon, 3 Apr 2017, 17:25 Wilson Page, notifications@github.com wrote:

You're right, browsers do batch up layout changes until the next render cycle. The real answer is that it all depends. It's perfectly safe to mutate layout triggering style properties so long as no other code requests to measure the DOM in the same sync task.

It depends how much you trust other engineers and third-party code in your app to do avoid this.

On Mon, 3 Apr 2017 10:15 Matthew Jacobi, notifications@github.com wrote:

I had the same thought as you but then I read http://kellegous.com/j/2013/01/26/layout-performance/

which claims that browsers use an on demand model of layout, and will avoid calculating layout until needed with an example

using fastdom and it's great, but trying to stop a brief flash of content due to a class not being applied fast enough I was hoping if I didn't have to queue the class change in the mutate phase that it would show sooner (and be ready for anything that runs in the measure phase)

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub <https://github.com/wilsonpage/fastdom/issues/103#issuecomment-291192712 , or mute the thread < https://github.com/notifications/unsubscribe-auth/AA-Sh2sM4cZauNnyY9pB3zs4z5r_fmDqks5rsRsKgaJpZM4MxYhD

.

— You are receiving this because you authored the thread.

Reply to this email directly, view it on GitHub https://github.com/wilsonpage/fastdom/issues/103#issuecomment-291195842, or mute the thread https://github.com/notifications/unsubscribe-auth/AAC91OM9lYiFLq6fAkEy7ieghdQDAx5aks5rsR2NgaJpZM4MxYhD .

wilsonpage commented 7 years ago

What about reading the class list? That's safe and doesn't need measure?

Reading a classList is fine as it's not requesting geometric data from the DOM.

I set the class and then measure.

If you have a element that depends on the natural height of another element you could hide both elements (using opacity: 0) and then reveal both once you have completed your custom layout. You should be able to finish the work in one frame.

aFarkas commented 7 years ago

@oppianmatt Can you Show some Code. I'm interested.