Nested media queries cause state update on unmounted component

[gersomvg]

The error can be reproduced when scaling down this codepen from above 720px to below 720px:

https://codepen.io/gersomvg/pen/yxQgBB

[edorivai]

I'm sorry, can you elaborate. What is happening for you, what do you expect to happen?

[gersomvg]

Just check the error in the console, when scaling down the window ;) Visually everything works as expected, but behind the screens React warns for changing state on an unmounted components.

The child component tries to update when the media query becomes falsey, but it is already being unmounted by the parent components that implements the same media query. Normally this should be prevented by a proper componentWillUnmount logic, but I think there is some race condition.

[edorivai]

I'm probably missing something, but I'm not getting any warnings.

[gersomvg]

We're getting somewhere. It looks like it's Safari specific. Maybe browsers have different timing, when it comes to firing events.

[edorivai]

Ah I see. Yeah so we're already removing the event listener for media query changes in componentWillUnmount. And since the only place that we setState is located within that event handler, I guess we can conclude that in Safari, the media query handler is called before componentWillUnmount, but after the component is actually unmounted 🤔.

This doesn't really make sense. I'm running Linux myself, so it's kinda hard for me to reproduce this. I would really appreciate if you could get some more info on what happens in which exact order.

[gersomvg]

Yeah, that's super weird. That would mean that for the child component it fires one cycle later than for the parent component.

I'll see what I can do here, debugging-wise...

[yiochen]

The parent and the child have the same media query. When the media query gets triggered, the engine pushes two events to the event queue to be executed. The first event calls parent's setState, which will unmount the child, the second event calls child's setState. Looks like Safari doesn't do a good job of clearing out events already in the queue, even if the event listener is removed. I wrote a simple test in jsbin:

var mediaQuery = '(min-width: 500px)';
var media = window.matchMedia(mediaQuery);
function listener1() {
  console.log('listener1');
  media.removeListener(listener2);
}
function listener2() {
  console.log('listener2');
}
media.addListener(listener1);
media.addListener(listener2);

When we increase the width to more then 500px, we should only see listener1 (verified with Chrome). But on Safari, I was able to see both listener1 and listener2.

http://jsbin.com/labaciqigu/edit?js,console,output

[edorivai]

@yiochen thanks for the write up, that makes a lot of sense.

Soooo I guess this is a Safari bug?

[gersomvg]

@yiochen Thanks indeed. I guess we can solve this easily, by making a thin wrapper around the addListener and removeListener that prevents this bug from happening. I'm happy to write a PR for this, if desired.

[edorivai]

PR's welcome :)

[JerryMissTom]

@edorivai @Gersom-NL I come across this bug in my computer. The same code in https://codepen.io/gersomvg/pen/yxQgBB , I resize the Chrome several times, then it will show error info in Console. The screenshot is below

My computer is Mac Pro, OS version is 10.14.6 (18G95), and Chrome version is 76.0.3809.87 (64 bit)