Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(react-router): lazyRouteComponent reload guard key fallsback to importer's import.meta.url on Safari #3450

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Sheraff
Copy link
Contributor

@Sheraff Sheraff commented Feb 16, 2025

Fixes #3327

This PR might be easier to review as commit-by-commit (because there are many changes in generated files packages/router-plugin/tests/code-splitter/snapshots/...).

  • e2e/react-router/basic-file-based-code-splitting
    Add an E2E playwright spec for validating the reload behavior of pages where a lazy import fails to load
  • packages/react-router/src/lazyRouteComponent.tsx
    Accept a 4th parameter to provide a unique key to the sessionStorage entry used as a reload guard on a failed lazy load
  • packages/router-generator/src/generator.ts and packages/router-plugin/src/core/code-splitter/compilers.ts
    Provide 4th parameter to lazyRouteComponent calls as import.meta.url. This is native in browsers, so should be "bundler independant" (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/import.meta).

Note

The playwright spec added by this PR tests a behavior that might be browser-specific. However, the E2E tests run on chromium only. Maybe it might be worth it to enable multiple "projects" in playwright?

See implementation:

function isModuleNotFoundError(error: any): boolean {
// chrome: "Failed to fetch dynamically imported module: http://localhost:5173/src/routes/posts.index.tsx?tsr-split"
// firefox: "error loading dynamically imported module: http://localhost:5173/src/routes/posts.index.tsx?tsr-split"
// safari: "Importing a module script failed."
if (typeof error?.message !== 'string') return false
return (
error.message.startsWith('Failed to fetch dynamically imported module') ||
error.message.startsWith('error loading dynamically imported module') ||
error.message.startsWith('Importing a module script failed')
)
}


In a future PR, it might be a good idea to apply the same (or similar) behavior on import fail to the lazyFn since it's used in a way that feels similar to lazyRouteComponent, but has no error handling.

// A function that takes an import() argument which is a function and returns a new function that will
// proxy arguments from the caller to the imported function, retaining all type
// information along the way
export function lazyFn<
T extends Record<string, (...args: Array<any>) => any>,
TKey extends keyof T = 'default',
>(fn: () => Promise<T>, key?: TKey) {
return async (
...args: Parameters<T[TKey]>
): Promise<Awaited<ReturnType<T[TKey]>>> => {
const imported = await fn()
return imported[key || 'default'](...args)
}
}

Copy link

nx-cloud bot commented Feb 16, 2025

View your CI Pipeline Execution ↗ for commit 5ffcb02.

Command Status Duration Result
nx affected --targets=test:eslint,test:unit,tes... ✅ Succeeded 4m 22s View ↗
nx run-many --target=build --exclude=examples/*... ✅ Succeeded 1m 23s View ↗

☁️ Nx Cloud last updated this comment at 2025-02-16 11:59:54 UTC

Copy link

pkg-pr-new bot commented Feb 16, 2025

Open in Stackblitz

More templates

@tanstack/arktype-adapter

npm i https://pkg.pr.new/@tanstack/arktype-adapter@3450

@tanstack/create-start

npm i https://pkg.pr.new/@tanstack/create-start@3450

@tanstack/create-router

npm i https://pkg.pr.new/@tanstack/create-router@3450

@tanstack/directive-functions-plugin

npm i https://pkg.pr.new/@tanstack/directive-functions-plugin@3450

@tanstack/eslint-plugin-router

npm i https://pkg.pr.new/@tanstack/eslint-plugin-router@3450

@tanstack/history

npm i https://pkg.pr.new/@tanstack/history@3450

@tanstack/react-cross-context

npm i https://pkg.pr.new/@tanstack/react-cross-context@3450

@tanstack/react-router

npm i https://pkg.pr.new/@tanstack/react-router@3450

@tanstack/react-router-with-query

npm i https://pkg.pr.new/@tanstack/react-router-with-query@3450

@tanstack/router-core

npm i https://pkg.pr.new/@tanstack/router-core@3450

@tanstack/router-cli

npm i https://pkg.pr.new/@tanstack/router-cli@3450

@tanstack/router-devtools

npm i https://pkg.pr.new/@tanstack/router-devtools@3450

@tanstack/router-generator

npm i https://pkg.pr.new/@tanstack/router-generator@3450

@tanstack/router-plugin

npm i https://pkg.pr.new/@tanstack/router-plugin@3450

@tanstack/router-utils

npm i https://pkg.pr.new/@tanstack/router-utils@3450

@tanstack/router-vite-plugin

npm i https://pkg.pr.new/@tanstack/router-vite-plugin@3450

@tanstack/server-functions-plugin

npm i https://pkg.pr.new/@tanstack/server-functions-plugin@3450

@tanstack/start

npm i https://pkg.pr.new/@tanstack/start@3450

@tanstack/start-api-routes

npm i https://pkg.pr.new/@tanstack/start-api-routes@3450

@tanstack/start-client

npm i https://pkg.pr.new/@tanstack/start-client@3450

@tanstack/start-config

npm i https://pkg.pr.new/@tanstack/start-config@3450

@tanstack/start-plugin

npm i https://pkg.pr.new/@tanstack/start-plugin@3450

@tanstack/start-router-manifest

npm i https://pkg.pr.new/@tanstack/start-router-manifest@3450

@tanstack/start-server

npm i https://pkg.pr.new/@tanstack/start-server@3450

@tanstack/start-server-functions-client

npm i https://pkg.pr.new/@tanstack/start-server-functions-client@3450

@tanstack/start-server-functions-fetcher

npm i https://pkg.pr.new/@tanstack/start-server-functions-fetcher@3450

@tanstack/start-server-functions-handler

npm i https://pkg.pr.new/@tanstack/start-server-functions-handler@3450

@tanstack/start-server-functions-server

npm i https://pkg.pr.new/@tanstack/start-server-functions-server@3450

@tanstack/valibot-adapter

npm i https://pkg.pr.new/@tanstack/valibot-adapter@3450

@tanstack/start-server-functions-ssr

npm i https://pkg.pr.new/@tanstack/start-server-functions-ssr@3450

@tanstack/virtual-file-routes

npm i https://pkg.pr.new/@tanstack/virtual-file-routes@3450

@tanstack/zod-adapter

npm i https://pkg.pr.new/@tanstack/zod-adapter@3450

commit: 5ffcb02

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auto-reload on failed lazy route loading can only work once per session on Safari
1 participant