Preact: heads up, latest preact-x with compat with next.js latest canary - infinite loop at app load

[srdjan]

Reproduction

using-preact next.js example with latest next canary and latest preact

Steps to reproduce

infinite loop at app start time, error:

Script terminated by timeout at:
preact__WEBPACK_IMPORTED_MODULE_0__.options.__b@webpack-internal:///./node_modules/preact/debug/dist/debug.module.js:5:5986
preact__WEBPACK_IMPORTED_MODULE_1__.options.__b@webpack-internal:///./node_modules/preact/compat/dist/compat.module.js:54:1062
T@webpack-internal:///./node_modules/preact/dist/preact.module.js:15:4554
g@webpack-internal:///./node_modules/preact/dist/preact.module.js:15:2155
T@webpack-internal:///./node_modules/preact/dist/preact.module.js:15:5984
m/<@webpack-internal:///./node_modules/preact/dist/preact.module.js:15:1509
m@webpack-internal:///./node_modules/preact/dist/preact.module.js:15:1409
preact__WEBPACK_IMPORTED_MODULE_0__.options.debounceRendering/<@webpack-internal:///./node_modules/@prefresh/core/src/runtime/debounceRendering.js:16:4
debug.module.js:1:4896

[cristianbote]

Hey @srdjan thanks for opening this issue. I'm trying to narrow it down but I failed. Can you share a repo/codesandbox with the above issue?

[marvinhagemeister]

We'd love to take a look at this but needs a reproduction case. Can you share a codesandbox or a repo where the issue can be reproduced?

[kevineinarsson]

I created a reproduction link over at: https://codesandbox.io/s/wild-microservice-7z30n?runonclick=1 - not sure if I managed getting codesandbox to install canary-versions of Next though. I think it "works" (breaks) as expected. Running the example crashes the tab.

I did some digging and found what I think is the issue. In canary-28, the Link component was converted to a function component, and in the Material UI example (which is what I'm using in my project) this updated link component gets sent as a reference to the MUI "link" component to be rendered inside their link component (for styling). Something with the references is causing an infinite loop(?). Pull that was merged is at https://github.com/vercel/next.js/pull/14633

This wrapping component that the MUI example uses is inside the example as well, src/Link.js

[srdjan]

link to repository with reproduction:
https://github.com/srdjan/next-repros

[marvinhagemeister]

Awesome thank you so much to you both for the reproductions! You two rock 🙌 Can't stress enough how much this makes it easier for us to fix bugs 💯

[cristianbote]

Just like @marvinhagemeister said! 😃

It's something related to <Link /> if you remove those from Navigation(https://github.com/srdjan/next-repros/blob/master/components/navigation.js#L10-L15) things are working fine.

Looking into it more.

[developit]

I'm thinking this might be caused by Next manually invoking ref() on the child passed to <Link>.

Or, it's the ref->state loop they have (I'm paraphrasing):

function Link(props) {
  const [childElm, setChildElm] = useState();

  const child = Children.only(props.children);

  const childProps = {
    ref: (el) => {
      setChildElm(el)
    }
  };

  return React.cloneElement(child, childProps);
}

For some reason we are re-calling ref() in this case, even though the element shouldn't be changing.

Another thing I wanted to point out - it seems like this is only happening during development? Maybe something we're doing in the debug add-on is triggering type changes that cause the ref to be re-invoked every time? (purely a guess)

[cristianbote]

I get that infinite loop reproducing on production build as well. Not happening with react so I'll take your pointers and dig more in a few.

Thanks @developit for weigh in on this one 👍

[developit]

I wonder if it's a memoization bail-out? Next uses transform-hoist-constant-elements, and the inner <a> inside <Link> is a constant element, so it's likely being hoisted.

They're passing a new ref function reference on every render, and in that ref callback they set a value in state, which likely triggers a new render. The thing is - ref callbacks that change on every render get called twice on every render. So that means this component is intentionally triggering this loop:

1. render the child with a new ref()
2. the renderer calls any previous ref with null
3. this triggers setChildElm(null)
4. the renderer calls the new ref with <a>
5. this triggers setChildElm(<a>)
6. Link is re-rendered due to the state change

Batching prevents #3 from triggering a re-render itself - but - calling setChildElm(null) and then setChildElm(<a>) is bypassing our duplicate update check:

// a working version:
setChildElm(<a>)  // sets state value to <a>
setChildElm(<a>)  // skipped - state value is already <a>

// failing version (their impl):
setChildElm(<a>)  // sets state value to <a>
setChildElm(null)  // sets state value to null  (null !== <a>)
setChildElm(<a>)  // sets state value to <a>  (<a> !== null)

I believe React uses a "settling" logic to drop repeated hook changes, whereas we use referential equality. Here, it seems like that breaks.

[cristianbote]

Alrighty, @developit was totally correct that was the issue. He opened up a PR against next/link to fix the aggressive state updates. Here's the PR https://github.com/vercel/next.js/pull/15068.

[developit]

The Next.js fix was landed in [email protected]. @srdjan if you're able to update, would love to know if that fixed it for you.

[srdjan]

yes, that. fixed it for me!