Fix stuck onboarding route on failed chunk preload (#23359)
Fixes [Sentry 7604159654](https://sentry.io/issues/7604159654/) (v2.20.0, Mobile Safari). The onboarding router preloads 7 lazy chunks on entry; Vite's CSS preload for SyncEmails rejected and three defects compounded: - `void SomePage.preload()` discarded the promise, so it became an unhandled rejection and the user got a raw `Unable to preload CSS for /assets/...css` snackbar. - `lazyWithPreload` cached the *rejected* promise and rendered via `throw preload()`. React pings on the rejection, re-renders, the component throws the same settled rejected thenable, the ping listener de-dupes, and the route hangs on its loader forever. - `checkIfItsAViteStaleChunkLazyLoadingError` only matched Chrome's message, so `AppErrorBoundary`'s reload recovery never fired for the CSS-preload or Safari variants. `lazyWithPreload` now records the failure in state instead of rethrowing, so the thenable thrown into Suspense always fulfills, `preload()` returns void and can never reject, and the render path throws the real `Error` to the boundary, which reloads. Two things worth knowing for review: `React.lazy` is not a substitute here (its initializer has no synchronous fast path, so it suspends even when the module is already loaded, reintroducing the loader flash #22392 removed), and the failure is deliberately sticky because Vite marks the dep `seen` before attempting it, so an in-document retry loads the JS without its CSS and silently renders an unstyled page. <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/23359?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
This commit is contained in:
@@ -124,13 +124,13 @@ const NotFound = lazy(() =>
|
||||
);
|
||||
|
||||
const preloadOnboardingPages = () => {
|
||||
void WorkspaceActivation.preload();
|
||||
void CreateProfile.preload();
|
||||
void SyncEmails.preload();
|
||||
void InstallApps.preload();
|
||||
void InviteTeam.preload();
|
||||
void ChooseYourPlan.preload();
|
||||
void WorkspaceSetup.preload();
|
||||
WorkspaceActivation.preload();
|
||||
CreateProfile.preload();
|
||||
SyncEmails.preload();
|
||||
InstallApps.preload();
|
||||
InviteTeam.preload();
|
||||
ChooseYourPlan.preload();
|
||||
WorkspaceSetup.preload();
|
||||
|
||||
return null;
|
||||
};
|
||||
|
||||
+28
@@ -11,6 +11,34 @@ describe('checkIfItsAViteStaleChunkLazyLoadingError', () => {
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it('should return true for the Firefox dynamic import failure message', () => {
|
||||
const error = new Error(
|
||||
'error loading dynamically imported module: /some/module.js',
|
||||
);
|
||||
|
||||
const result = checkIfItsAViteStaleChunkLazyLoadingError(error);
|
||||
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it('should return true for the Safari dynamic import failure message', () => {
|
||||
const error = new Error('Importing a module script failed.');
|
||||
|
||||
const result = checkIfItsAViteStaleChunkLazyLoadingError(error);
|
||||
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it('should return true when a CSS chunk fails to preload', () => {
|
||||
const error = new Error(
|
||||
'Unable to preload CSS for /assets/SyncEmails-DKxn4rm-.css',
|
||||
);
|
||||
|
||||
const result = checkIfItsAViteStaleChunkLazyLoadingError(error);
|
||||
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it('should return false when error message does not contain the Vite stale chunk error text', () => {
|
||||
const error = new Error('Some other error message');
|
||||
|
||||
|
||||
+10
-1
@@ -1,3 +1,12 @@
|
||||
const VITE_STALE_CHUNK_ERROR_MESSAGES = [
|
||||
'Failed to fetch dynamically imported module',
|
||||
'error loading dynamically imported module',
|
||||
'Importing a module script failed',
|
||||
'Unable to preload CSS for',
|
||||
];
|
||||
|
||||
export const checkIfItsAViteStaleChunkLazyLoadingError = (error: Error) => {
|
||||
return error.message.includes('Failed to fetch dynamically imported module');
|
||||
return VITE_STALE_CHUNK_ERROR_MESSAGES.some((staleChunkErrorMessage) =>
|
||||
error.message.includes(staleChunkErrorMessage),
|
||||
);
|
||||
};
|
||||
|
||||
@@ -0,0 +1,184 @@
|
||||
import { render, screen } from '@testing-library/react';
|
||||
import { Suspense, type ComponentType } from 'react';
|
||||
import { ErrorBoundary, type FallbackProps } from 'react-error-boundary';
|
||||
|
||||
import { lazyWithPreload } from '~/utils/lazyWithPreload';
|
||||
|
||||
const PRELOAD_ERROR_MESSAGE =
|
||||
'Unable to preload CSS for /assets/SyncEmails-DKxn4rm-.css';
|
||||
|
||||
const PageContent = () => <div>page content</div>;
|
||||
|
||||
type ErrorFallbackProps = FallbackProps;
|
||||
|
||||
const ErrorFallback = ({ error }: ErrorFallbackProps) => (
|
||||
<div>{error.message}</div>
|
||||
);
|
||||
|
||||
const createDeferredLoader = () => {
|
||||
let resolveModule!: (loadedModule: { default: ComponentType }) => void;
|
||||
let rejectModule!: (error: Error) => void;
|
||||
|
||||
const modulePromise = new Promise<{ default: ComponentType }>(
|
||||
(resolve, reject) => {
|
||||
resolveModule = resolve;
|
||||
rejectModule = reject;
|
||||
},
|
||||
);
|
||||
|
||||
return {
|
||||
loader: jest.fn(() => modulePromise),
|
||||
resolveModule: () => resolveModule({ default: PageContent }),
|
||||
rejectModule: () => rejectModule(new Error(PRELOAD_ERROR_MESSAGE)),
|
||||
};
|
||||
};
|
||||
|
||||
const flushPendingPromises = () =>
|
||||
new Promise<void>((resolve) => setTimeout(resolve, 0));
|
||||
|
||||
describe('lazyWithPreload', () => {
|
||||
let consoleErrorSpy: jest.SpyInstance;
|
||||
|
||||
beforeEach(() => {
|
||||
consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
consoleErrorSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('should not produce an unhandled rejection when the preload fails', async () => {
|
||||
const onUnhandledRejection = jest.fn();
|
||||
process.on('unhandledRejection', onUnhandledRejection);
|
||||
|
||||
try {
|
||||
const { loader, rejectModule } = createDeferredLoader();
|
||||
const Component = lazyWithPreload(loader);
|
||||
|
||||
Component.preload();
|
||||
rejectModule();
|
||||
await flushPendingPromises();
|
||||
} finally {
|
||||
process.off('unhandledRejection', onUnhandledRejection);
|
||||
}
|
||||
|
||||
expect(onUnhandledRejection).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should not retry the loader once the preload has failed', async () => {
|
||||
const { loader, rejectModule } = createDeferredLoader();
|
||||
const Component = lazyWithPreload(loader);
|
||||
|
||||
Component.preload();
|
||||
rejectModule();
|
||||
await flushPendingPromises();
|
||||
Component.preload();
|
||||
|
||||
expect(loader).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('should treat a synchronous loader throw as a failed load instead of throwing from preload', async () => {
|
||||
const loader = jest.fn(() => {
|
||||
throw new Error(PRELOAD_ERROR_MESSAGE);
|
||||
});
|
||||
const Component = lazyWithPreload(loader);
|
||||
|
||||
expect(() => Component.preload()).not.toThrow();
|
||||
await flushPendingPromises();
|
||||
|
||||
render(
|
||||
<ErrorBoundary FallbackComponent={ErrorFallback}>
|
||||
<Suspense fallback={<div>loading</div>}>
|
||||
<Component />
|
||||
</Suspense>
|
||||
</ErrorBoundary>,
|
||||
);
|
||||
|
||||
expect(screen.getByText(PRELOAD_ERROR_MESSAGE)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('should call the loader once across repeated preloads', () => {
|
||||
const { loader } = createDeferredLoader();
|
||||
const Component = lazyWithPreload(loader);
|
||||
|
||||
Component.preload();
|
||||
Component.preload();
|
||||
|
||||
expect(loader).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('should render without ever showing the suspense fallback once preloaded', async () => {
|
||||
const { loader, resolveModule } = createDeferredLoader();
|
||||
const Component = lazyWithPreload(loader);
|
||||
const Fallback = jest.fn(() => <div>loading</div>);
|
||||
|
||||
Component.preload();
|
||||
resolveModule();
|
||||
await flushPendingPromises();
|
||||
|
||||
render(
|
||||
<Suspense fallback={<Fallback />}>
|
||||
<Component />
|
||||
</Suspense>,
|
||||
);
|
||||
|
||||
expect(screen.getByText('page content')).toBeInTheDocument();
|
||||
expect(Fallback).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should show the fallback then the component when rendered before the load completes', async () => {
|
||||
const { loader, resolveModule } = createDeferredLoader();
|
||||
const Component = lazyWithPreload(loader);
|
||||
|
||||
render(
|
||||
<Suspense fallback={<div>loading</div>}>
|
||||
<Component />
|
||||
</Suspense>,
|
||||
);
|
||||
|
||||
expect(screen.getByText('loading')).toBeInTheDocument();
|
||||
|
||||
resolveModule();
|
||||
|
||||
expect(await screen.findByText('page content')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('should throw the load error to the error boundary when rendered after a failed preload', async () => {
|
||||
const { loader, rejectModule } = createDeferredLoader();
|
||||
const Component = lazyWithPreload(loader);
|
||||
|
||||
Component.preload();
|
||||
rejectModule();
|
||||
await flushPendingPromises();
|
||||
|
||||
render(
|
||||
<ErrorBoundary FallbackComponent={ErrorFallback}>
|
||||
<Suspense fallback={<div>loading</div>}>
|
||||
<Component />
|
||||
</Suspense>
|
||||
</ErrorBoundary>,
|
||||
);
|
||||
|
||||
expect(screen.getByText(PRELOAD_ERROR_MESSAGE)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('should leave the fallback for the error boundary when the load fails while suspended', async () => {
|
||||
const { loader, rejectModule } = createDeferredLoader();
|
||||
const Component = lazyWithPreload(loader);
|
||||
|
||||
render(
|
||||
<ErrorBoundary FallbackComponent={ErrorFallback}>
|
||||
<Suspense fallback={<div>loading</div>}>
|
||||
<Component />
|
||||
</Suspense>
|
||||
</ErrorBoundary>,
|
||||
);
|
||||
|
||||
expect(screen.getByText('loading')).toBeInTheDocument();
|
||||
|
||||
rejectModule();
|
||||
|
||||
expect(await screen.findByText(PRELOAD_ERROR_MESSAGE)).toBeInTheDocument();
|
||||
expect(screen.queryByText('loading')).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
@@ -1,31 +1,65 @@
|
||||
import { type ComponentType } from 'react';
|
||||
|
||||
type LoadState =
|
||||
| { status: 'idle' }
|
||||
| { status: 'pending'; promise: Promise<void> }
|
||||
| { status: 'loaded'; component: ComponentType }
|
||||
| { status: 'failed'; error: unknown };
|
||||
|
||||
type PreloadableComponent = ComponentType & {
|
||||
preload: () => Promise<void>;
|
||||
preload: () => void;
|
||||
};
|
||||
|
||||
export const lazyWithPreload = (
|
||||
loader: () => Promise<{ default: ComponentType }>,
|
||||
): PreloadableComponent => {
|
||||
let LoadedComponent: ComponentType | null = null;
|
||||
let loadingPromise: Promise<void> | null = null;
|
||||
let loadState: LoadState = { status: 'idle' };
|
||||
|
||||
const startLoading = (): Promise<void> => {
|
||||
if (loadState.status === 'pending') {
|
||||
return loadState.promise;
|
||||
}
|
||||
|
||||
if (loadState.status !== 'idle') {
|
||||
return Promise.resolve();
|
||||
}
|
||||
|
||||
try {
|
||||
const promise = loader().then(
|
||||
(loadedModule) => {
|
||||
loadState = { status: 'loaded', component: loadedModule.default };
|
||||
},
|
||||
(error) => {
|
||||
loadState = { status: 'failed', error };
|
||||
},
|
||||
);
|
||||
|
||||
loadState = { status: 'pending', promise };
|
||||
|
||||
return promise;
|
||||
} catch (error) {
|
||||
loadState = { status: 'failed', error };
|
||||
|
||||
return Promise.resolve();
|
||||
}
|
||||
};
|
||||
|
||||
const preload = () => {
|
||||
loadingPromise ??= loader().then((loadedModule) => {
|
||||
LoadedComponent = loadedModule.default;
|
||||
});
|
||||
|
||||
return loadingPromise;
|
||||
startLoading();
|
||||
};
|
||||
|
||||
const PreloadableComponent = () => {
|
||||
const Component = LoadedComponent;
|
||||
|
||||
if (Component === null) {
|
||||
throw preload();
|
||||
if (loadState.status === 'failed') {
|
||||
throw loadState.error;
|
||||
}
|
||||
|
||||
return <Component />;
|
||||
if (loadState.status === 'loaded') {
|
||||
const Component = loadState.component;
|
||||
|
||||
return <Component />;
|
||||
}
|
||||
|
||||
throw startLoading();
|
||||
};
|
||||
|
||||
return Object.assign(PreloadableComponent, { preload });
|
||||
|
||||
Reference in New Issue
Block a user