Mastodon: unobserve() of IntersectionObserver breaks Edge support

[unarist]

We've started to use IntersectionObserver#unobserve() at #3851, but this causes TypeMismatchError on Edge and prevents further script execution (e.g. back to Getting Started from Local timeline).

I found some similar reports (unobserve() causes TypeMismatchError) by googling, so I think it would be a Edge's bug.

Microsoft Edge 40.15063.0.0
Microsoft EdgeHTML 15.15063

---

• [x] I searched or browsed the repo’s other issues to ensure this is not a duplicate.
• [master] This bug happens on a tagged release and not on master (If you're a user, don't worry about this).

[beatrix-bitrot]

@nolanlawson any thoughts on this?

[sorin-davidoi]

I'll take a look.

[sorin-davidoi]

I'd say we revert #3851. Unobserving the statuses, when they are unmounted, does not make sense, since they get unmounted when StatusList is unmounted, which destroys its IntersectionObserver.

[nolanlawson]

Thanks for the fix, and if you get a chance, please file a bug on Edge. :) We're working right now on making our IntersectionObserver implementation more compatible with Chrome's (and soon, Firefox's).

[nolanlawson]

Also that particular fix was just fixing a React warning, so I would say we ignore the warning. Calling setState after unmount is due to how the requestIdleCallback wrapper is written and results in a no-op. So it makes the most sense just to ignore the warning IMO.

[unarist]

I found actual cause.

const observer = new IntersectionObserver(()=>{});
observer.unobserve(document.body);

This code calls unobserve() for the element which is not observed yet. Chrome and Firefox ignore this, but Edge throws TypeMismatchError.

And we unobserve same target twice on column unmounting:

• Unmounting StatusList calls disconnect() (= unobserve all targets)
• Unmounting Status calls unobserve()

...so TypeMismatchError will be thrown on Edge.

Also I think this issue may have occured on rapid scrolling like this:

const observer = new IntersectionObserver(()=>{});

// good
observer.observe(document.body);
observer.unobserve(document.body);
observer.observe(document.body);
observer.unobserve(document.body);

// bad
observer.observe(document.body);
observer.observe(document.body);
observer.unobserve(document.body);
observer.unobserve(document.body);

---

Well, if elements will be deleted after unmounting, we don't need to unobserve manually. Spec also says it's addresses lazy-loading.

So simply ignoring the warning makes sense, and we have #3853 which adds componentMounted flag and avoid setState() using it. I think it would covers most cases.

[nolanlawson]

Looks like you filed the issue? https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/12577586/ :smiley: Thanks a lot, and my team will look into it!