hydration of pre-rendered lazy route / component triggers exception, corrupts component tree

[danielweck]

Describe the bug

Exception TypeError: e.__k is null is raised when attempting to hydrate a pre-rendered lazy route / dynamically-imported component. The error doesn't occur when hydrating a non-lazy route first, then navigating to a lazy one.

To Reproduce

1. npm init wmr lazy-hydrate-error
2. cd lazy-hydrate-error
3. open public/index.js, add onError prop e.g. : <ErrorBoundary onError={(e) => { console.log('ErrorBoundary onError: ', e); }}>
4. npm run build && npm run serve
5. load http://192.168.1.127:8080/about in web browser + open inspector
6. CTRL/CMD+R / F5 to hard-refresh the page
7. observe TypeError: e.__k is null => somewhere downstream of the setState() call chain ... seems to be update(1) in the lazy wrapper component:

   wmr/packages/preact-iso/lazy.js

   Line 11 in 928c7a0

   if (!r.current) r.current = p.then(() => update(1));

Here is another method to quickly reproduce the bug, from your local copy of the WMR repository:

• in the main branch, run yarn demo serve
• open http://localhost:8080/lazy-and-late
• wait 2-3 seconds, then navigate to any non-lazy route (i.e. any other route in the navigation header)
• => OKAY :)
• now hard-refresh http://localhost:8080/lazy-and-late with CMD+R to start over again
• DO NOT WAIT 2-3 seconds, instead navigate to any non-lazy route (i.e. any other route in the navigation header) relatively quickly (in my tests, the bug occurs even if I take my time to load another route ... let's say 1s)
• => BREAK :(

Expected behavior

No error should occur.

Desktop (please complete the following information):

• OS: all
• Browser: all
• WMR Version: all

Additional context

This behaviour is problematic when needing to leverage onError to handle error state, see for example this related issue: #423

[danielweck]

Hello, I have added a screenshot that shows the 100% reproducible exception somewhere downstream of a setState() call chain.

[danielweck]

Using the web inspector / debugger, I have narrowed it down to the update(1) call in the lazy wrapper component:

wmr/packages/preact-iso/lazy.js

Line 11 in 928c7a0

if (!r.current) r.current = p.then(() => update(1));

[danielweck]

If I comment out this update(1) section:

wmr/packages/preact-iso/lazy.js

Line 11 in 928c7a0

if (!r.current) r.current = p.then(() => update(1));

...then the exception occurs at the update(0) call in the Router's commit function:

wmr/packages/preact-iso/router.js

Lines 109 to 114 in 928c7a0

	const commit = () => {
	if (cur.current.url !== url || pending.current !== p) return;
	prev.current = prevChildren.current = pending.current = null;
	if (props.onLoadEnd) props.onLoadEnd(url);
	update(0);
	};

[danielweck]

What is your method to debug into Preact's internals? (sourcemaps? if so, how to enable them in WMR's prerender build mode?)

If I understand correctly, some code makes use of reserved minified properties (**), so aliasing preact and preact/hooks to the ESM code would break things. I used this alias config in my package.json to debug into WMR's code:

	"alias": {
		"preact-iso/router": "/PATH/TO/wmr/packages/preact-iso/router",
		"preact-iso/hydrate": "/PATH/TO/wmr/packages/preact-iso/hydrate",
		"preact-iso/lazy": "/PATH/TO/wmr/packages/preact-iso/lazy",
		"__preact__": "/PATH/TO/preact/src",
		"__preact/hooks__": "/PATH/TO/preact/hooks/src"

	},

(**) For example __d for _dirty:

wmr/packages/preact-iso/lazy.js

Line 22 in 24417f6

if (err && err.then) this.__d = true;

https://github.com/preactjs/preact/blob/7556b61bd0d9f72bbd906c3abaebf2b1c9aeacbf/mangle.json#L40

"$_dirty": "__d",

[danielweck]

Here is another data point to help troubleshoot this problem:

The exception occurs as soon as the lazy / dynamically-imported component is loaded. By adding an artificial loading delay, and by navigating to a different router path during the load, it is possible to reliably reproduce a broken VNode/DOM tree (i.e. the lazy component is displayed at the same time as the new route). Once again, this only occurs in prerender / production mode (npm run build && npm run serve), not in development (npm start)

const AboutLate = lazy(() => import('./pages/about/index.js'));

==>

const AboutLate = lazy(
	() =>
		new Promise((resolve) => {
			setTimeout(() => {
				resolve(import('./pages/about/index.js'));
			}, 2000);
		}),
);

[danielweck]

In order to prevent the fatal error described in the message above (which really is a deal breaker for using async routes / lazy components at the moment in WMR), I am using the following workaround which prevents user attempts to trigger other routes while an async route is loading (i.e. lazy component not yet tree-mounted):

EDIT: I use a CSS cursor "not allowed" on router links that do not respond to clicks whilst async route is loading, and I use a CSS cursor "wait" on the link that triggered the lazy component.

	useEffect(() => {

		const clickHandler = (ev) => {
			if (!ev.target) {
				return;
			}
			const linkEl = ev.target.closest('a[href]');

			if (!linkEl || linkEl.origin !== window.location.origin) {
				return;
			}

			if (isLoading) {
				ev.stopPropagation();
				ev.preventDefault();
			}

			// this is just a handy feature to test full server reload (not necessary for this workaround)
			if (ev.altKey) {
				ev.stopPropagation(); // prevents preact-iso router (see comment above)
				ev.preventDefault(); // prevents default linking / popup menu behaviour
				window.location.href = linkEl.href;
			}
		};

		document.addEventListener('click', clickHandler, {
			capture: true,
		});

		return () => {
			document.removeEventListener('click', clickHandler, {
				capture: true,
			});
		};
	}, [isLoading]);

As the bug only occurs with pre-rendered static SSR (npm run build --prerender), it is possible to only apply this workaround in that mode, and not in development mode (i.e. npm run start). However the discrepancy in behaviour would be confusing, so I apply the workaround everywhere.

As you can see, this technique relies on a boolean variable isLoading, which reflects the router state. More on that in the message below.

[danielweck]

Here's how isLoading reflects the router state:

const ctx = {
	routerLoading: (isLoading: boolean) => {
		//
	},
};
const ContextRouterLoading = createContext(ctx);
ContextRouterLoading.displayName = 'Router Loading Context';

export const App = () => {
	const [isLoading, setLoading] = useState(false);
	const ctx = {
		routerLoading: (isLoading) => {

			// https://github.com/preactjs/wmr/issues/425
			if (!!document.querySelector('script[type=isodata]') // pre-rendered
				&& !window.__LAZY_CHECK) {
				window.__LAZY_CHECK = true;
				if (isLoading) {
					return;
				}
			}
			setLoading(isLoading);
		},
	};

	useEffect(() => {

		const clickHandler = (ev) => {
			if (!ev.target) {
				return;
			}
			const linkEl = ev.target.closest('a[href]');

			if (!linkEl || linkEl.origin !== window.location.origin) {
				return;
			}

			if (isLoading) {
				ev.stopPropagation();
				ev.preventDefault();
				return;
			}

			if (ev.altKey) {
				ev.stopPropagation(); // prevents preact-iso router (see comment above)
				ev.preventDefault(); // prevents default linking / popup menu behaviour
				window.location.href = linkEl.href;
				return;
			}
		};

		document.addEventListener('click', clickHandler, {
			capture: true,
		});

		return () => {
			document.removeEventListener('click', clickHandler, {
				capture: true,
			});
		};
	}, [isLoading]);

	return (
		<ContextRouterLoading.Provider value={ctx}>
			<LocationProvider>
				<div>
					<Header loading={isLoading} />
					<ErrorBoundary
						onError={(err) => {
							console.log('ErrorBoundary onError: ', err);
						}}
					>
						<Router
							onLoadStart={
								(url) => {
									ctx.routerLoading(true);
								}
							}
							onLoadEnd={
								(url) => {
									ctx.routerLoading(false);
								}
							}
						>
							<RouteWrapper path="/">
								<Home />
							</RouteWrapper>
							<RouteWrapper path="/about-late">
								<AboutLate />
							</RouteWrapper>
							<RouteWrapper default>
								<NotFound />
							</RouteWrapper>
						</Router>
					</ErrorBoundary>
				</div>
			</LocationProvider>
		</ContextRouterLoading.Provider>
	);
};

Finally, note that the <RouteWrapper> children of <Router> are necessary in my use-case to work around the problem of "error boundary", as reported separately in this issue:

#423 (comment)

function RouteWrapper(props) {
    const [error, setError] = useState(undefined);

	this.componentDidCatch = (err) => {
		if (!err.then) {
			setError(err);
		}
	};

	if (error) {

		return (
			<>
				<p>ERROR!</p>
				<p>{error.message}</p>
				<button
					onClick={() => {
						setError(undefined);
					}}
				>
					Try again
				</button>
			</>
		);
	}

	return props.children;
}

I hope this helps :)

[danielweck]

Ah, another behaviour I've noticed is that the exception occurs shortly after hydration, but not instantaneously. There is a short time window during which the user can click on a routed link, resulting in corrupting the component tree. Thankfully that was easy to solve as I already have code in place to check hydration status. I just added a 500ms timeout to provide enough time for the exception to occur, beyond which point the other workarounds detailed above are effective. Phew! :)

[danielweck]

I have been running tests based on the stream of tweaks / fixes in preact-iso/router, notably:
#504
and
#525
and
#364
and
#358

Unfortunately, I am still able to reproduce this bug consistently. Thankfully I am successfully using several workarounds to prevent the bug's occurrence, but the resulting code is quite convoluted. Just to be clear: the TypeError: e.__k is null error cannot just be ignored (it is by default swallowed by the ErrorBoundary), as it seems to be related to some underlying state corruption, as described in the comment thread above.

[danielweck]

Here is another method to quickly reproduce the bug, from your local copy of the WMR repository:

• in the main branch, run yarn demo serve
• open http://localhost:8080/lazy-and-late
• wait 2-3 seconds, then navigate to any non-lazy route (i.e. any other route in the navigation header)
• => OKAY :)
• now hard-refresh http://localhost:8080/lazy-and-late with CMD+R to start over again
• DO NOT WAIT 2-3 seconds, instead navigate to any non-lazy route (i.e. any other route in the navigation header) relatively quickly (in my tests, the bug occurs even if I take my time to load another route ... let's say 1s)
• => BREAK :(

[developit]

This should have been fixed by #525.