From fa691b16c907cab0cad4c223a4f441faab295c37 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Thu, 1 Feb 2024 15:24:49 +0000 Subject: [PATCH] feat(remix): Add dynamic loading of `@remix-run/router` module for `matchRoutes`. --- packages/remix/src/utils/instrumentServer.ts | 8 ++- packages/remix/src/utils/routerLoader.ts | 51 +++++++++++++++++++ .../remix/src/utils/serverAdapters/express.ts | 14 ++--- packages/remix/test/integration/package.json | 10 ++-- 4 files changed, 66 insertions(+), 17 deletions(-) create mode 100644 packages/remix/src/utils/routerLoader.ts diff --git a/packages/remix/src/utils/instrumentServer.ts b/packages/remix/src/utils/instrumentServer.ts index 94e8090ac433..df5ce88eed8e 100644 --- a/packages/remix/src/utils/instrumentServer.ts +++ b/packages/remix/src/utils/instrumentServer.ts @@ -29,6 +29,7 @@ import { import { DEBUG_BUILD } from './debug-build'; import { getFutureFlagsServer, getRemixVersionFromBuild } from './futureFlags'; +import { loadRemixRouterModule } from './routerLoader'; import { extractData, getRequestMatch, @@ -59,6 +60,8 @@ import { normalizeRemixRequest } from './web-fetch'; let FUTURE_FLAGS: FutureConfig | undefined; let IS_REMIX_V2: boolean | undefined; +let pkg: ReactRouterDomPkg | undefined; + const redirectStatusCodes = new Set([301, 302, 303, 307, 308]); function isRedirectResponse(response: Response): boolean { return redirectStatusCodes.has(response.status); @@ -447,7 +450,6 @@ export function getTransactionName( function wrapRequestHandler(origRequestHandler: RequestHandler, build: ServerBuild): RequestHandler { const routes = createRoutes(build.routes); - const pkg = loadModule('react-router-dom'); return async function (this: unknown, request: RemixRequest, loadContext?: AppLoadContext): Promise { // This means that the request handler of the adapter (ex: express) is already wrapped. @@ -456,6 +458,10 @@ function wrapRequestHandler(origRequestHandler: RequestHandler, build: ServerBui return origRequestHandler.call(this, request, loadContext); } + if (!pkg) { + pkg = await loadRemixRouterModule(); + } + return runWithAsyncContext(async () => { // eslint-disable-next-line deprecation/deprecation const hub = getCurrentHub(); diff --git a/packages/remix/src/utils/routerLoader.ts b/packages/remix/src/utils/routerLoader.ts new file mode 100644 index 000000000000..3be8ce71286b --- /dev/null +++ b/packages/remix/src/utils/routerLoader.ts @@ -0,0 +1,51 @@ +import { loadModule, logger } from '@sentry/utils'; +import { cwd } from 'process'; +import { DEBUG_BUILD } from './debug-build'; +import type { ReactRouterDomPkg } from './vendor/types'; + +function hasMatchRoutes(pkg: ReactRouterDomPkg | undefined): boolean { + return !!pkg && typeof pkg.matchRoutes === 'function'; +} + +/** + * Loads the router package that provides matchRoutes. + * Tries loading @remix-run/router first, then falls back to react-router-dom. + */ +export async function loadRemixRouterModule(): Promise { + // Try loading @remix-run/router first, then fall back to react-router-dom + for (const moduleName of ['@remix-run/router', 'react-router-dom']) { + const pkg = await tryLoadRouterModule(moduleName); + + if (pkg) { + return pkg; + } + } + + DEBUG_BUILD && logger.warn('Could not find a router package that provides `matchRoutes`.'); + + return; +} + +async function tryLoadRouterModule(moduleName: string): Promise { + let pkg: ReactRouterDomPkg | undefined; + + pkg = loadModule(moduleName); + + if (hasMatchRoutes(pkg)) { + return pkg; + } + + try { + pkg = await import(moduleName); + } catch (e) { + pkg = await import(`${cwd()}/node_modules/${moduleName}`); + } + + if (hasMatchRoutes(pkg)) { + return pkg; + } else { + DEBUG_BUILD && logger.warn(`Could not find ${moduleName} package.`); + } + + return; +} diff --git a/packages/remix/src/utils/serverAdapters/express.ts b/packages/remix/src/utils/serverAdapters/express.ts index 8f05de780381..96bd9ae5e6ea 100644 --- a/packages/remix/src/utils/serverAdapters/express.ts +++ b/packages/remix/src/utils/serverAdapters/express.ts @@ -9,10 +9,10 @@ import { import { flush } from '@sentry/node'; import type { Transaction } from '@sentry/types'; import { extractRequestData, fill, isString, logger } from '@sentry/utils'; -import { cwd } from 'process'; import { DEBUG_BUILD } from '../debug-build'; import { createRoutes, getTransactionName, instrumentBuild, startRequestHandlerTransaction } from '../instrumentServer'; +import { loadRemixRouterModule } from '../routerLoader'; import type { AppLoadContext, ExpressCreateRequestHandler, @@ -26,7 +26,7 @@ import type { ServerBuild, } from '../vendor/types'; -let pkg: ReactRouterDomPkg; +let pkg: ReactRouterDomPkg | undefined; function wrapExpressRequestHandler( origRequestHandler: ExpressRequestHandler, @@ -41,15 +41,7 @@ function wrapExpressRequestHandler( next: ExpressNextFunction, ): Promise { if (!pkg) { - try { - pkg = await import('react-router-dom'); - } catch (e) { - pkg = await import(`${cwd()}/node_modules/react-router-dom`); - } finally { - if (!pkg) { - DEBUG_BUILD && logger.error('Could not find `react-router-dom` package.'); - } - } + pkg = await loadRemixRouterModule(); } await runWithAsyncContext(async () => { diff --git a/packages/remix/test/integration/package.json b/packages/remix/test/integration/package.json index 03068150e6f8..cd4cde7f2224 100644 --- a/packages/remix/test/integration/package.json +++ b/packages/remix/test/integration/package.json @@ -7,16 +7,16 @@ "start": "remix-serve build" }, "dependencies": { - "@remix-run/express": "1.17.0", - "@remix-run/node": "1.17.0", - "@remix-run/react": "1.17.0", - "@remix-run/serve": "1.17.0", + "@remix-run/express": "1.19.3", + "@remix-run/node": "1.19.3", + "@remix-run/react": "1.19.3", + "@remix-run/serve": "1.19.3", "@sentry/remix": "file:../..", "react": "^17.0.2", "react-dom": "^17.0.2" }, "devDependencies": { - "@remix-run/dev": "1.17.0", + "@remix-run/dev": "1.19.3", "@types/react": "^17.0.47", "@types/react-dom": "^17.0.17", "nock": "^13.1.0",