From 618b7d77e7ab6ee2c7c5a333f89c161e17262526 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Mon, 26 Feb 2024 14:45:46 +0000 Subject: [PATCH] feat(remix): Add Vite dev-mode support to Express instrumentation. (#10784) Resolves #10724 Related: https://github.com/getsentry/sentry-javascript/issues/9500 Adds dev-mode support to Remix setups with Vite and Express. We currently accept Remix server `build` as an object to instrument. But Remix allows `build` as a synchronous or asynchronous function that returns the build object. Currently, it seems that functions are only used in development servers, and not in production. So, while this update slightly reduces `requestHandler` performance on dev servers, it does not on production builds. We need `build` in 2 places: 1- We instrument the loaders / actions on build, then we pass them down to the original implementations. 2- We use the `routes` inside them to create parameterised transactions. This update adds new internal wrappers around them to make sure that we don't miss out on the returned / resolved values in case `build` is a function, for both cases. This PR also adds a new E2E test application using the latest Remix version and Vite, and it runs the tests on `dev` mode. We also need a documentation update to reflect this, if it gets merged. --- .github/workflows/build.yml | 1 + .../.eslintrc.js | 79 + .../.gitignore | 5 + .../create-remix-app-express-vite-dev/.npmrc | 2 + .../README.md | 34 + .../app/entry.client.tsx | 46 + .../app/entry.server.tsx | 147 + .../app/root.tsx | 80 + .../app/routes/_index.tsx | 21 + .../app/routes/client-error.tsx | 13 + .../app/routes/navigate.tsx | 20 + .../app/routes/user.$id.tsx | 3 + .../env.d.ts | 2 + .../package.json | 51 + .../playwright.config.ts | 58 + .../server.mjs | 53 + .../tests/behaviour-client.test.ts | 200 + .../tsconfig.json | 24 + .../vite.config.ts | 18 + .../yarn.lock | 6230 +++++++++++++++++ .../create-remix-app-v2/package.json | 12 +- .../remix/src/utils/serverAdapters/express.ts | 102 +- 22 files changed, 7179 insertions(+), 22 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/.eslintrc.js create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/.gitignore create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/.npmrc create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/README.md create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/app/entry.client.tsx create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/app/entry.server.tsx create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/app/root.tsx create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/app/routes/_index.tsx create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/app/routes/client-error.tsx create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/app/routes/navigate.tsx create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/app/routes/user.$id.tsx create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/env.d.ts create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/package.json create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/playwright.config.ts create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/server.mjs create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/tests/behaviour-client.test.ts create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/tsconfig.json create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/vite.config.ts create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/yarn.lock diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 52ad9f46570f..f6ac07ee2532 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -1057,6 +1057,7 @@ jobs: 'create-next-app', 'create-remix-app', 'create-remix-app-v2', + 'create-remix-app-express-vite-dev', 'debug-id-sourcemaps', 'nextjs-app-dir', 'nextjs-14', diff --git a/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/.eslintrc.js b/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/.eslintrc.js new file mode 100644 index 000000000000..e0a82f1826e3 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/.eslintrc.js @@ -0,0 +1,79 @@ +/** + * This is intended to be a basic starting point for linting in your app. + * It relies on recommended configs out of the box for simplicity, but you can + * and should modify this configuration to best suit your team's needs. + */ + +/** @type {import('eslint').Linter.Config} */ +module.exports = { + root: true, + parserOptions: { + ecmaVersion: 'latest', + sourceType: 'module', + ecmaFeatures: { + jsx: true, + }, + }, + env: { + browser: true, + commonjs: true, + es6: true, + }, + + // Base config + extends: ['eslint:recommended'], + + overrides: [ + // React + { + files: ['**/*.{js,jsx,ts,tsx}'], + plugins: ['react', 'jsx-a11y'], + extends: [ + 'plugin:react/recommended', + 'plugin:react/jsx-runtime', + 'plugin:react-hooks/recommended', + 'plugin:jsx-a11y/recommended', + ], + settings: { + react: { + version: 'detect', + }, + formComponents: ['Form'], + linkComponents: [ + { name: 'Link', linkAttribute: 'to' }, + { name: 'NavLink', linkAttribute: 'to' }, + ], + 'import/resolver': { + typescript: {}, + }, + }, + }, + + // Typescript + { + files: ['**/*.{ts,tsx}'], + plugins: ['@typescript-eslint', 'import'], + parser: '@typescript-eslint/parser', + settings: { + 'import/internal-regex': '^~/', + 'import/resolver': { + node: { + extensions: ['.ts', '.tsx'], + }, + typescript: { + alwaysTryTypes: true, + }, + }, + }, + extends: ['plugin:@typescript-eslint/recommended', 'plugin:import/recommended', 'plugin:import/typescript'], + }, + + // Node + { + files: ['.eslintrc.js', 'server.mjs'], + env: { + node: true, + }, + }, + ], +}; diff --git a/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/.gitignore b/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/.gitignore new file mode 100644 index 000000000000..80ec311f4ff5 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/.gitignore @@ -0,0 +1,5 @@ +node_modules + +/.cache +/build +.env diff --git a/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/.npmrc b/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/.npmrc new file mode 100644 index 000000000000..070f80f05092 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/.npmrc @@ -0,0 +1,2 @@ +@sentry:registry=http://127.0.0.1:4873 +@sentry-internal:registry=http://127.0.0.1:4873 diff --git a/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/README.md b/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/README.md new file mode 100644 index 000000000000..ec619a8eb455 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/README.md @@ -0,0 +1,34 @@ +# Welcome to Remix + Vite! + +📖 See the [Remix docs](https://remix.run/docs) and the [Remix Vite docs](https://remix.run/docs/en/main/future/vite) for details on supported features. + +## Development + +Run the Express server with Vite dev middleware: + +```shellscript +npm run dev +``` + +## Deployment + +First, build your app for production: + +```sh +npm run build +``` + +Then run the app in production mode: + +```sh +npm start +``` + +Now you'll need to pick a host to deploy it to. + +### DIY + +If you're familiar with deploying Express applications you should be right at home. Just make sure to deploy the output of `npm run build` + +- `build/server` +- `build/client` diff --git a/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/app/entry.client.tsx b/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/app/entry.client.tsx new file mode 100644 index 000000000000..d71aaa5cd286 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/app/entry.client.tsx @@ -0,0 +1,46 @@ +import { RemixBrowser, useLocation, useMatches } from '@remix-run/react'; +import * as Sentry from '@sentry/remix'; +import { StrictMode, startTransition, useEffect } from 'react'; +import { hydrateRoot } from 'react-dom/client'; + +Sentry.init({ + environment: 'qa', // dynamic sampling bias to keep transactions + dsn: window.ENV.SENTRY_DSN, + integrations: [ + Sentry.browserTracingIntegration({ + useEffect, + useLocation, + useMatches, + }), + new Sentry.Replay(), + ], + // Performance Monitoring + tracesSampleRate: 1.0, // Capture 100% of the transactions, reduce in production! + // Session Replay + replaysSessionSampleRate: 0.1, // This sets the sample rate at 10%. You may want to change it to 100% while in development and then sample at a lower rate in production. + replaysOnErrorSampleRate: 1.0, // If you're not already sampling the entire session, change the sample rate to 100% when sampling sessions where errors occur. +}); + +Sentry.addEventProcessor(event => { + if ( + event.type === 'transaction' && + (event.contexts?.trace?.op === 'pageload' || event.contexts?.trace?.op === 'navigation') + ) { + const eventId = event.event_id; + if (eventId) { + window.recordedTransactions = window.recordedTransactions || []; + window.recordedTransactions.push(eventId); + } + } + + return event; +}); + +startTransition(() => { + hydrateRoot( + document, + + + , + ); +}); diff --git a/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/app/entry.server.tsx b/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/app/entry.server.tsx new file mode 100644 index 000000000000..c3deb6369af3 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/app/entry.server.tsx @@ -0,0 +1,147 @@ +import { PassThrough } from 'node:stream'; + +import type { AppLoadContext, EntryContext } from '@remix-run/node'; +import { createReadableStreamFromReadable } from '@remix-run/node'; +import { installGlobals } from '@remix-run/node'; +import { RemixServer } from '@remix-run/react'; +import * as Sentry from '@sentry/remix'; +import * as isbotModule from 'isbot'; +import { renderToPipeableStream } from 'react-dom/server'; + +installGlobals(); + +const ABORT_DELAY = 5_000; + +Sentry.init({ + environment: 'qa', // dynamic sampling bias to keep transactions + dsn: process.env.E2E_TEST_DSN, + // Performance Monitoring + tracesSampleRate: 1.0, // Capture 100% of the transactions, reduce in production! +}); + +export const handleError = Sentry.wrapRemixHandleError; + +export default function handleRequest( + request: Request, + responseStatusCode: number, + responseHeaders: Headers, + remixContext: EntryContext, + loadContext: AppLoadContext, +) { + return isBotRequest(request.headers.get('user-agent')) + ? handleBotRequest(request, responseStatusCode, responseHeaders, remixContext) + : handleBrowserRequest(request, responseStatusCode, responseHeaders, remixContext); +} + +// We have some Remix apps in the wild already running with isbot@3 so we need +// to maintain backwards compatibility even though we want new apps to use +// isbot@4. That way, we can ship this as a minor Semver update to @remix-run/dev. +function isBotRequest(userAgent: string | null) { + if (!userAgent) { + return false; + } + + // isbot >= 3.8.0, >4 + if ('isbot' in isbotModule && typeof isbotModule.isbot === 'function') { + return isbotModule.isbot(userAgent); + } + + // isbot < 3.8.0 + if ('default' in isbotModule && typeof isbotModule.default === 'function') { + return isbotModule.default(userAgent); + } + + return false; +} + +function handleBotRequest( + request: Request, + responseStatusCode: number, + responseHeaders: Headers, + remixContext: EntryContext, +) { + return new Promise((resolve, reject) => { + let shellRendered = false; + const { pipe, abort } = renderToPipeableStream( + , + { + onAllReady() { + shellRendered = true; + const body = new PassThrough(); + const stream = createReadableStreamFromReadable(body); + + responseHeaders.set('Content-Type', 'text/html'); + + resolve( + new Response(stream, { + headers: responseHeaders, + status: responseStatusCode, + }), + ); + + pipe(body); + }, + onShellError(error: unknown) { + reject(error); + }, + onError(error: unknown) { + responseStatusCode = 500; + // Log streaming rendering errors from inside the shell. Don't log + // errors encountered during initial shell rendering since they'll + // reject and get logged in handleDocumentRequest. + if (shellRendered) { + console.error(error); + } + }, + }, + ); + + setTimeout(abort, ABORT_DELAY); + }); +} + +function handleBrowserRequest( + request: Request, + responseStatusCode: number, + responseHeaders: Headers, + remixContext: EntryContext, +) { + return new Promise((resolve, reject) => { + let shellRendered = false; + const { pipe, abort } = renderToPipeableStream( + , + { + onShellReady() { + shellRendered = true; + const body = new PassThrough(); + const stream = createReadableStreamFromReadable(body); + + responseHeaders.set('Content-Type', 'text/html'); + + resolve( + new Response(stream, { + headers: responseHeaders, + status: responseStatusCode, + }), + ); + + pipe(body); + }, + onShellError(error: unknown) { + reject(error); + }, + onError(error: unknown) { + responseStatusCode = 500; + // Log streaming rendering errors from inside the shell. Don't log + // errors encountered during initial shell rendering since they'll + // reject and get logged in handleDocumentRequest. + if (shellRendered) { + console.error(error); + } + }, + }, + ); + + setTimeout(abort, ABORT_DELAY); + }); +} diff --git a/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/app/root.tsx b/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/app/root.tsx new file mode 100644 index 000000000000..517a37a9d76b --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/app/root.tsx @@ -0,0 +1,80 @@ +import { cssBundleHref } from '@remix-run/css-bundle'; +import { LinksFunction, MetaFunction, json } from '@remix-run/node'; +import { + Links, + LiveReload, + Meta, + Outlet, + Scripts, + ScrollRestoration, + useLoaderData, + useRouteError, +} from '@remix-run/react'; +import { captureRemixErrorBoundaryError, withSentry } from '@sentry/remix'; +import type { SentryMetaArgs } from '@sentry/remix'; + +export const links: LinksFunction = () => [...(cssBundleHref ? [{ rel: 'stylesheet', href: cssBundleHref }] : [])]; + +export const loader = () => { + return json({ + ENV: { + SENTRY_DSN: process.env.E2E_TEST_DSN, + }, + }); +}; + +export const meta = ({ data }: SentryMetaArgs>) => { + return [ + { + env: data.ENV, + }, + { + name: 'sentry-trace', + content: data.sentryTrace, + }, + { + name: 'baggage', + content: data.sentryBaggage, + }, + ]; +}; + +export function ErrorBoundary() { + const error = useRouteError(); + const eventId = captureRemixErrorBoundaryError(error); + + return ( +
+ ErrorBoundary Error + {eventId} +
+ ); +} + +function App() { + const { ENV } = useLoaderData(); + + return ( + + + + +