Preact Components not unmounting due to InstantClick page lifecycle events.

[nickytonline]

Describe the bug

This can manifest itself in different ways, but a good example is the home feed keyboard navigation. Every time you leave the home screen and come back the keyboard shortcuts stack meaning the next button (J), which should advance by 1, advances by 2, 3 or n posts per click.

Previous investigation showed that the problem is that Preact components are not properly unmounting when a user leaves the page, due to InstantClick behaviour.

Previous notes from Nick:

I know how to fix it @reobin. I will create an issue and put a PR up for it. We need to explicitly call unmountComponentAtNode in an InstantClick.on('change', () => {}) event. This is something that normally we wouldn't have to do, but because we use InstantClick, it's necessary. It also explains why I saw components repeated in the Preact dev tools.

I need to do it for pretty much all out components that get mounted via render. e.g.

import { h, render } from 'preact';
import { unmountComponentAtNode } from 'preact/compat';
import { Search } from '../Search';
import 'focus-visible';

document.addEventListener('DOMContentLoaded', () => {
  const root = document.getElementById('header-search');

  render(<Search />, root);

  InstantClick.on('change', () => {
    unmountComponentAtNode(root);
  });
});

To Reproduce

This will be having effects in multiple places that might not be quite so visible, but as above, you can see the impact on the home page.

• Visit the home feed and use the j key to go to the next post - verify it steps through posts one by one
• Visit and article and then return to the home feed
• Use j to go to the next post in the feed - notice it 'skips' a post
• Repeat the above and observe that the shortcut skips n posts depending how many times you have navigated to/from the feed

Expected behavior

Preact components should be unmounted when an InstantClick even changes the page.

In the example above this would mean j always advances one post at a time, regardless of previous navigation.

Screenshots

See #11458 (comment)

[github-actions]

Thanks for the issue! We'll take your request into consideration and follow up if we decide to tackle this issue.

To our amazing contributors: issues labeled type: bug are always up for grabs, but for feature requests, please wait until we add a ready for dev before starting to work on it.

To claim an issue to work on, please leave a comment. If you've claimed the issue and need help, please ping @forem/oss and we will follow up within 3 business days.

For full info on how to contribute, please check out our contributors guide.

[Link2Twenty]

@nickytonline is there a solution to this in the pipeline? I see the pull request you submitted was closed for more thought.

[nickytonline]

My initial try for this kind of worked. I need to dig into this more still. Also other things took priority, so this completely fell off my radar. 😅 @cmgorton, this would be good to add to the next cycle work.

[Link2Twenty]

@Ridhwana @aitchiss is this on anyone's radar? Who is the best person to ping about this now?

[aitchiss]

Hey @Link2Twenty, no, it's not currently prioritised. I noticed the two bugs referenced by this issue have been closed - do you know if this behaviour is causing bugs in the live app at the moment? I'm just thinking it might be helpful to reframe this issue around unwanted behaviours in production, so we can see what priority level it should be.

[Link2Twenty]

The main one that I keep noticing is keyboard shortcut jumping.

Every time you leave the home screen and come back the keyboard shortcuts stack meaning the next button (J), which should advance by 1, advances by 2, 3 or n posts per click.

When the page first loads

After a bit of navigation

[aitchiss]

OK great - thank you @Link2Twenty! Checking the notes on #11371 now it seems like we closed it in favour of this issue, but then this issue was never fully resolved 🙃

I'll update the description here and add the bug label and pop it in our queue of issues to review