preact icon indicating copy to clipboard operation
preact copied to clipboard

10.18.2 regression: Cannot read properties of undefined

Open zakstucke opened this issue 2 years ago • 6 comments

  • [ ] Check if updating to the latest Preact version resolves the issue

Describe the bug Our dashboard starts fails when upgrading from 18.1 to 18.2. Looks like suspense boundary regressions. I haven't got an easy repro or capacity to find one right now, just opening for visibility on the issue.

Screenshot 2023-11-06 at 13 23 22

zakstucke avatar Nov 06 '23 13:11 zakstucke

@zakstucke any luck on the reproduction yet?

JoviDeCroock avatar Dec 29 '23 07:12 JoviDeCroock

@JoviDeCroock sad to say not yet! Like with the last suspense issue I helped with a while back, they seem to pop up in very specific circumstances and so all seems to work fine in a simple repo...

I've just checked with the latest version: 10.19.3 where issue still persists, last working version 10.18.1.

image

zakstucke avatar Jan 05 '24 06:01 zakstucke

Although looking at this new screenshot errs seem slightly different, now in signals, although this might be unrelated and from changes in my code.

zakstucke avatar Jan 05 '24 06:01 zakstucke

@zakstucke I wonder if it's https://github.com/preactjs/preact/pull/4182/files we should check for the existence of oldVNode where does the stacktrace lead you in preact? or it has something to do with _nextDom setting with the insertBefore warning... not sure

JoviDeCroock avatar Jan 11 '24 13:01 JoviDeCroock

@JoviDeCroock @marvinhagemeister sorry to say that fix wasn't it!

But I've been useless for long enough, I've spent some time this morning trimming code until I got to a simple repro.

Repro made with npm preact init can be found here: https://github.com/zakstucke/preact-4194-repro

This repro fails with 10.19.5, works with 10.18.1, and pretty sure it's the root of this issue.

cd my-preact-app && npm i && npm run dev

Going to http://localhost:5173 will give the following error: image

The repro is effectively:

import { render } from 'preact';
import { Suspense } from "react";

import { Tab } from "@headlessui/react";

import { TestComp, TestWrapper } from "./lazyBarrier";

export function App() {
	return (
		<Suspense fallback={<p>Loading...</p>}>
			<Tab.Group>
				<Tab>
					<p>Foo</p>
				</Tab>
				<Tab.Panels>
					<Tab.Panel>
						<TestWrapper>
							<TestComp />
						</TestWrapper>
					</Tab.Panel>
				</Tab.Panels>
			</Tab.Group>
		</Suspense>
	);
}

render(<App />, document.getElementById('app'));

Where TestWrapper is a lazy loaded wrapper component (just returning children) and TestComp is also lazy loaded and just returns <p>Bar</p>

I haven't been able to get @headlessui/react out of the mix, but it should be a start to diagnose!

Thanks

zakstucke avatar Feb 17 '24 09:02 zakstucke

Hmm, I think that narrows it down to https://github.com/preactjs/preact/pull/4182, after some testing it does not seem to be the aforementioned issue. 4182 is a pretty big release so will look into this when I have some more time.

JoviDeCroock avatar Feb 17 '24 12:02 JoviDeCroock

Okay, I had some time today to get to the bottom of this...

The main issue is that in nested fragment-like children we can have a very deep chain of components where the last one eventually resolves to unmounting the children.

This effectively means that _nextDom on all parents is going to be invalid, we however can't course-correct as we've already higher-up in the stack assigned oldDom https://github.com/preactjs/preact/blob/main/src/diff/children.js#L61 which now in-turn can become invalid by the first call to diff https://github.com/preactjs/preact/blob/main/src/diff/children.js#L85

JoviDeCroock avatar Mar 20 '24 11:03 JoviDeCroock

@JoviDeCroock I can confirm this actually fixed it! Thanks so much for all your help :)

zakstucke avatar Apr 12 '24 10:04 zakstucke