From c49e7b1019a5673906f603d8e5f51192996f1847 Mon Sep 17 00:00:00 2001 From: Fran Dios Date: Fri, 5 Apr 2024 13:39:39 +0200 Subject: [PATCH 01/13] Fix cross-package types --- packages/cli/src/commands/hydrogen/dev-vite.ts | 7 +++---- packages/cli/tsconfig.json | 6 +----- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/packages/cli/src/commands/hydrogen/dev-vite.ts b/packages/cli/src/commands/hydrogen/dev-vite.ts index 25f6aa685f..7bdcc6c9d1 100644 --- a/packages/cli/src/commands/hydrogen/dev-vite.ts +++ b/packages/cli/src/commands/hydrogen/dev-vite.ts @@ -36,10 +36,9 @@ import {getCliCommand} from '../../lib/shell.js'; import {findPort} from '../../lib/find-port.js'; import {logRequestLine} from '../../lib/mini-oxygen/common.js'; -// @ts-ignore -- Module outside of the rootDir -import type {OxygenApiOptions} from '~/mini-oxygen/vite/plugin.js'; -// @ts-ignore -- Module outside of the rootDir -import type {HydrogenPluginOptions} from '~/hydrogen/vite/plugin.js'; +// Do not import JS from here, only types +import type {OxygenApiOptions} from '@shopify/mini-oxygen/vite'; +import type {HydrogenPluginOptions} from '@shopify/hydrogen/vite'; export default class DevVite extends Command { static description = diff --git a/packages/cli/tsconfig.json b/packages/cli/tsconfig.json index c44ac6ccac..b2a0812875 100644 --- a/packages/cli/tsconfig.json +++ b/packages/cli/tsconfig.json @@ -9,10 +9,6 @@ "rootDir": "src", "noUncheckedIndexedAccess": true, "types": ["@shopify/oxygen-workers-types", "node", "@remix-run/dev"], - "outDir": "dist", - "paths": { - "~/mini-oxygen/*": ["../mini-oxygen/src/*"], - "~/hydrogen/*": ["../hydrogen/src/*"] - } + "outDir": "dist" } } From a06963281339b9f57f4b96c1c2028925ec9f2698 Mon Sep 17 00:00:00 2001 From: Fran Dios Date: Fri, 5 Apr 2024 13:45:49 +0200 Subject: [PATCH 02/13] Minor fixes to build types --- packages/cli/package.json | 2 +- packages/cli/tsup.config.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cli/package.json b/packages/cli/package.json index 2f89a203d4..60586e6953 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -79,7 +79,7 @@ "exports": { "./package.json": "./package.json", "./commands/hydrogen/init": { - "types": "./dist/commands/hydrogen/init.d.ts", + "types": "./dist/init.d.ts", "default": "./dist/commands/hydrogen/init.js" } }, diff --git a/packages/cli/tsup.config.ts b/packages/cli/tsup.config.ts index 2ae78ca409..34c118e42b 100644 --- a/packages/cli/tsup.config.ts +++ b/packages/cli/tsup.config.ts @@ -28,7 +28,7 @@ const outDir = 'dist'; export default defineConfig([ { ...commonConfig, - entry: ['src/**/*.ts'], + entry: ['src/**/*.ts', '!src/**/*.test.ts'], outDir, // Generate types only for the exposed entry points dts: {entry: ['src/commands/hydrogen/init.ts']}, From f3ebfca67f5b1e249dcb4f5f3fdf29bfa0bf8a4c Mon Sep 17 00:00:00 2001 From: Fran Dios Date: Fri, 5 Apr 2024 13:50:42 +0200 Subject: [PATCH 03/13] Fix virtual route test for classic compiler --- packages/cli/src/lib/virtual-routes.test.ts | 70 +++++++++++++++++++++ packages/cli/src/lib/virtual-routes.ts | 5 +- 2 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 packages/cli/src/lib/virtual-routes.test.ts diff --git a/packages/cli/src/lib/virtual-routes.test.ts b/packages/cli/src/lib/virtual-routes.test.ts new file mode 100644 index 0000000000..f1f3409fd3 --- /dev/null +++ b/packages/cli/src/lib/virtual-routes.test.ts @@ -0,0 +1,70 @@ +import {describe, it, expect} from 'vitest'; +import {fileURLToPath} from 'node:url'; +import path from 'node:path'; +import type {RemixConfig} from '@remix-run/dev/dist/config.js'; +import { + addVirtualRoutes, + VIRTUAL_ROOT, + VIRTUAL_ROUTES_DIR, +} from './virtual-routes.js'; + +const appDirectory = path.join( + path.dirname(fileURLToPath(import.meta.url)), + 'virtual-test', +); + +describe('virtual routes', () => { + it('adds virtual routes', async () => { + const config = { + appDirectory, + routes: {}, + } as RemixConfig; + + await addVirtualRoutes(config); + + expect(config.routes[VIRTUAL_ROOT]).toMatchObject({ + path: '', + id: VIRTUAL_ROOT, + file: expect.stringContaining('virtual-routes/virtual-root.jsx'), + }); + + expect(config.routes[VIRTUAL_ROUTES_DIR + '/index']).toMatchObject({ + parentId: VIRTUAL_ROOT, + path: undefined, + file: expect.stringContaining(VIRTUAL_ROUTES_DIR + '/index.tsx'), + }); + + expect(config.routes[VIRTUAL_ROUTES_DIR + '/graphiql']).toMatchObject({ + parentId: VIRTUAL_ROOT, + path: 'graphiql', + file: expect.stringContaining(VIRTUAL_ROUTES_DIR + '/graphiql.tsx'), + }); + }); + + it('skips existing routes', async () => { + const existingIndexRoute = { + id: 'routes/index', + index: true, + parentId: 'root', + path: undefined, + file: 'user-app/routes/index.tsx', + }; + + const config = { + appDirectory, + routes: { + [existingIndexRoute.id]: existingIndexRoute, + }, + } as unknown as RemixConfig; + + await addVirtualRoutes(config); + + expect(config.routes[existingIndexRoute.id]).toMatchObject( + existingIndexRoute, + ); + + expect(config.routes[VIRTUAL_ROUTES_DIR + '/index']).toBeFalsy(); + + expect(Object.values(config.routes).length).toBeGreaterThan(2); + }); +}); diff --git a/packages/cli/src/lib/virtual-routes.ts b/packages/cli/src/lib/virtual-routes.ts index 486e3694e8..7d6b353706 100644 --- a/packages/cli/src/lib/virtual-routes.ts +++ b/packages/cli/src/lib/virtual-routes.ts @@ -20,8 +20,11 @@ type MinimalRemixConfig = { export async function addVirtualRoutes( config: T, ): Promise { + const distPath = process.env.SHOPIFY_UNIT_TEST + ? fileURLToPath(new URL('../../../hydrogen/src/vite', import.meta.url)) + : fileURLToPath(new URL('..', import.meta.url)); + const userRouteList = Object.values(config.routes); - const distPath = fileURLToPath(new URL('..', import.meta.url)); const virtualRoutesPath = joinPath(distPath, VIRTUAL_ROUTES_DIR); for (const absoluteFilePath of await glob( From 0178b51896a5f438c75a9d56dacaa42b3d2f172b Mon Sep 17 00:00:00 2001 From: Fran Dios Date: Fri, 5 Apr 2024 13:57:00 +0200 Subject: [PATCH 04/13] Fix type --- packages/mini-oxygen/src/vite/server-middleware.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/mini-oxygen/src/vite/server-middleware.ts b/packages/mini-oxygen/src/vite/server-middleware.ts index aef33c18b5..37224a90fc 100644 --- a/packages/mini-oxygen/src/vite/server-middleware.ts +++ b/packages/mini-oxygen/src/vite/server-middleware.ts @@ -9,8 +9,6 @@ const scriptPath = fileURLToPath(new URL('./worker-entry.js', import.meta.url)); const FETCH_MODULE_PATHNAME = '/__vite_fetch_module'; const WARMUP_PATHNAME = '/__vite_warmup'; -type Binding = (...args: unknown[]) => unknown | Promise | void; - export type InternalMiniOxygenOptions = { /** * Creates bindings in `env` that can be used to fetch external services. @@ -33,11 +31,13 @@ export type InternalMiniOxygenOptions = { * Function that is stringified and runs in the worker. * It gets the binding function as its first argument. */ - script: (binding: Binding) => void; + script: ( + binding: (...args: unknown[]) => Promise, + ) => void; /** * The binding function that runs in the parent process. */ - binding: Binding; + binding: (...args: unknown[]) => unknown | Promise | void; } >; }; From 08f9498b87a84db8d856b873322ab42487d5124b Mon Sep 17 00:00:00 2001 From: Fran Dios Date: Fri, 5 Apr 2024 13:58:01 +0200 Subject: [PATCH 05/13] Add subrequest profiler logger globally in Node environments --- packages/hydrogen/src/vite/plugin.ts | 80 +++++++++++++++------------- 1 file changed, 44 insertions(+), 36 deletions(-) diff --git a/packages/hydrogen/src/vite/plugin.ts b/packages/hydrogen/src/vite/plugin.ts index 724a4585d3..6c1827a9d2 100644 --- a/packages/hydrogen/src/vite/plugin.ts +++ b/packages/hydrogen/src/vite/plugin.ts @@ -66,44 +66,52 @@ export function hydrogen(pluginOptions: HydrogenPluginOptions = {}): Plugin[] { (plugin) => plugin.name === 'oxygen:main', ); - oxygenPlugin?.api?.registerPluginOptions?.({ - shouldStartRuntime: () => !isRemixChildCompiler(resolvedConfig), - crossBoundarySetup: [ - /** - * To avoid initial CSS flash during development, - * most frameworks implement a way to gather critical CSS. - * Remix does this by calling a global function that their - * Vite plugin creates in the Node.js process: - * @see https://github.com/remix-run/remix/blob/b07921efd5e8eed98e2996749852777c71bc3e50/packages/remix-server-runtime/dev.ts#L37-L47 - * - * Here we are setting up a stub function in the Oxygen worker - * that will be called by Remix during development. Then, we forward - * this request to the Node.js process (Vite server) where the actual - * Remix function is called and the critical CSS is returned to the worker. - */ - { - script: (binding) => { - // Setup global dev hooks in Remix in the worker environment - // using the binding function passed from Node environment: - globalThis.__remix_devServerHooks = {getCriticalCss: binding}; + if (oxygenPlugin) { + oxygenPlugin.api?.registerPluginOptions?.({ + shouldStartRuntime: () => !isRemixChildCompiler(resolvedConfig), + crossBoundarySetup: [ + /** + * To avoid initial CSS flash during development, + * most frameworks implement a way to gather critical CSS. + * Remix does this by calling a global function that their + * Vite plugin creates in the Node.js process: + * @see https://github.com/remix-run/remix/blob/b07921efd5e8eed98e2996749852777c71bc3e50/packages/remix-server-runtime/dev.ts#L37-L47 + * + * Here we are setting up a stub function in the Oxygen worker + * that will be called by Remix during development. Then, we forward + * this request to the Node.js process (Vite server) where the actual + * Remix function is called and the critical CSS is returned to the worker. + */ + { + script: (binding) => { + // Setup global dev hooks in Remix in the worker environment + // using the binding function passed from Node environment: + globalThis.__remix_devServerHooks = {getCriticalCss: binding}; + }, + binding: (...args) => { + // Call the global Remix dev hook for critical CSS in Node environment: + return globalThis.__remix_devServerHooks?.getCriticalCss?.( + ...args, + ); + }, }, - binding: (...args) => { - // Call the global Remix dev hook for critical CSS in Node environment: - return globalThis.__remix_devServerHooks?.getCriticalCss?.( - ...args, - ); + { + script: (binding) => { + globalThis.__H2O_LOG_EVENT = binding; + }, + binding: (data) => { + emitRequestEvent(data, resolvedConfig.root); + }, }, - }, - { - script: (binding) => { - globalThis.__H2O_LOG_EVENT = binding; - }, - binding: (data) => { - emitRequestEvent(data, resolvedConfig.root); - }, - }, - ], - } satisfies OxygenApiOptions); + ], + } satisfies OxygenApiOptions); + } else { + // If Oxygen is not present, we are probably running + // on Node.js, so we can setup the global functions directly. + globalThis.__H2O_LOG_EVENT = (data) => { + emitRequestEvent(data, resolvedConfig.root); + }; + } }, configureServer(viteDevServer) { if (isRemixChildCompiler(viteDevServer.config)) return; From dbe7d5c063b8712d8e83c1ebd5a4016d15d6b924 Mon Sep 17 00:00:00 2001 From: Fran Dios Date: Fri, 5 Apr 2024 16:27:30 +0200 Subject: [PATCH 06/13] Improve types and split logic of mini-oxygen handler --- packages/cli/src/lib/mini-oxygen/workerd.ts | 2 +- packages/cli/src/lib/request-events.ts | 3 +- .../mini-oxygen/src/vite/server-middleware.ts | 2 +- packages/mini-oxygen/src/worker/e2e.test.ts | 2 +- packages/mini-oxygen/src/worker/handler.ts | 61 ++++++++++++---- packages/mini-oxygen/src/worker/index.ts | 69 +++++++++++++------ 6 files changed, 99 insertions(+), 40 deletions(-) diff --git a/packages/cli/src/lib/mini-oxygen/workerd.ts b/packages/cli/src/lib/mini-oxygen/workerd.ts index 69409e164d..ca62fc3969 100644 --- a/packages/cli/src/lib/mini-oxygen/workerd.ts +++ b/packages/cli/src/lib/mini-oxygen/workerd.ts @@ -57,10 +57,10 @@ export async function startWorkerdServer({ const miniOxygen = createMiniOxygen({ debug, - logRequestLine, port: appPort, host: 'localhost', liveReload: watch, + requestHook: logRequestLine, inspectorPort: publicInspectorPort, inspectWorkerName: mainWorkerName, assets: {port: assetsPort, directory: buildPathClient}, diff --git a/packages/cli/src/lib/request-events.ts b/packages/cli/src/lib/request-events.ts index 50b97b6331..8e43089ddf 100644 --- a/packages/cli/src/lib/request-events.ts +++ b/packages/cli/src/lib/request-events.ts @@ -30,8 +30,9 @@ export function setConstructors(constructors: { export const DEV_ROUTES = new Set([ '/graphiql', + '/graphiql/customer-account.schema.json', '/subrequest-profiler', - '/__vite_warmup', + '/debug-network-server', ]); type RequestEvent = { diff --git a/packages/mini-oxygen/src/vite/server-middleware.ts b/packages/mini-oxygen/src/vite/server-middleware.ts index 37224a90fc..c372afa1d4 100644 --- a/packages/mini-oxygen/src/vite/server-middleware.ts +++ b/packages/mini-oxygen/src/vite/server-middleware.ts @@ -76,7 +76,7 @@ export async function startMiniOxygenRuntime({ const miniOxygen = createMiniOxygen({ debug, inspectorPort, - logRequestLine, + requestHook: null, workers: [ { name: 'vite-env', diff --git a/packages/mini-oxygen/src/worker/e2e.test.ts b/packages/mini-oxygen/src/worker/e2e.test.ts index b662ce32e6..41053cede2 100644 --- a/packages/mini-oxygen/src/worker/e2e.test.ts +++ b/packages/mini-oxygen/src/worker/e2e.test.ts @@ -300,7 +300,7 @@ function withFixtures( }, ], sourceMapPath: path.join(tmpDir, relativeWorkerEntry + '.map'), - logRequestLine: null, + requestHook: null, } satisfies MiniOxygenOptions; const miniOxygen = createMiniOxygen(miniOxygenOptions); diff --git a/packages/mini-oxygen/src/worker/handler.ts b/packages/mini-oxygen/src/worker/handler.ts index 9d6e5b49f4..994d44462f 100644 --- a/packages/mini-oxygen/src/worker/handler.ts +++ b/packages/mini-oxygen/src/worker/handler.ts @@ -1,14 +1,22 @@ +// This file is stringified, do not import anything here. + type Service = {fetch: typeof fetch}; -export async function miniOxygenHandler( +export type MiniOxygenHandlerEnv = { + entry: Service; + assets?: Service; + hook?: Service; + staticAssetExtensions?: string[]; + oxygenHeadersMap: Record; +}; + +export function getMiniOxygenHandlerScript() { + return `export default { fetch: ${miniOxygenHandler} }\n${withRequestHook}`; +} + +async function miniOxygenHandler( request: Request, - env: { - entry: Service; - assets?: Service; - logRequest: Service; - staticAssetExtensions?: string[]; - oxygenHeadersMap: Record; - }, + env: MiniOxygenHandlerEnv, context: ExecutionContext, ) { const {pathname} = new URL(request.url); @@ -40,23 +48,46 @@ export async function miniOxygenHandler( }, } satisfies RequestInit; + return env.hook + ? withRequestHook({ + request: new Request(request, requestInit), + handler: env.entry, + hook: env.hook, + context, + }) + : env.entry.fetch(request, requestInit); +} + +type RequestHookOptions = { + handler: Service; + request: Request; + context: ExecutionContext; + hook?: Service; +}; + +export async function withRequestHook({ + request, + handler, + hook, + context, +}: RequestHookOptions) { const startTimeMs = Date.now(); - const response = await env.entry.fetch(request, requestInit); + const response = await handler.fetch(request); const durationMs = Date.now() - startTimeMs; - context.waitUntil( - env.logRequest.fetch( - new Request(request.url, { + if (hook) { + context.waitUntil( + hook.fetch(request.url, { method: request.method, signal: request.signal, headers: { - ...requestInit.headers, + ...Object.fromEntries(request.headers.entries()), 'o2-duration-ms': String(durationMs), 'o2-response-status': String(response.status), }, }), - ), - ); + ); + } return response; } diff --git a/packages/mini-oxygen/src/worker/index.ts b/packages/mini-oxygen/src/worker/index.ts index 6b17a5d9a0..2ad61f1043 100644 --- a/packages/mini-oxygen/src/worker/index.ts +++ b/packages/mini-oxygen/src/worker/index.ts @@ -16,7 +16,10 @@ import { buildAssetsUrl, createAssetsServer, } from './assets.js'; -import {miniOxygenHandler} from './handler.js'; +import { + getMiniOxygenHandlerScript, + type MiniOxygenHandlerEnv, +} from './handler.js'; import {OXYGEN_HEADERS_MAP} from '../common/headers.js'; import {findPort} from '../common/find-port.js'; import {OXYGEN_COMPAT_PARAMS} from '../common/compat.js'; @@ -64,7 +67,7 @@ export type MiniOxygenOptions = InputMiniflareOptions & { debug?: boolean; sourceMapPath?: string; assets?: AssetOptions; - logRequestLine?: LogRequestLine | null; + requestHook?: RequestHook | null; inspectWorkerName?: string; }; @@ -75,12 +78,12 @@ export function createMiniOxygen({ inspectorPort, assets, sourceMapPath = '', - logRequestLine, + requestHook, inspectWorkerName, ...miniflareOptions }: MiniOxygenOptions) { const mf = new Miniflare( - buildMiniflareOptions(miniflareOptions, logRequestLine, assets), + buildMiniflareOptions(miniflareOptions, requestHook, assets), ); if (!sourceMapPath) { @@ -155,7 +158,7 @@ export function createMiniOxygen({ mf.setOptions( buildMiniflareOptions( {...miniflareOptions, ...newOptions}, - logRequestLine, + requestHook, assets, ), ), @@ -169,12 +172,12 @@ export function createMiniOxygen({ }; } -type LogRequestLine = ( +type RequestHook = ( request: Request, opt: {responseStatus: number; durationMs: number}, -) => void; +) => void | Promise; -const defaultLogRequestLine: LogRequestLine = (request, {responseStatus}) => { +const defaultRequestHook: RequestHook = (request, {responseStatus}) => { console.log( `${request.method} ${responseStatus} ${request.url.replace( new URL(request.url).origin, @@ -208,7 +211,7 @@ const UNSAFE_OUTBOUND_SERVICE = { function buildMiniflareOptions( {workers, ...mfOverwriteOptions}: InputMiniflareOptions, - logRequestLine: LogRequestLine | null = defaultLogRequestLine, + requestHook: RequestHook | null = defaultRequestHook, assetsOptions?: AssetOptions, ): OutputMiniflareOptions { const entryWorker = workers.find((worker) => !!worker.name); @@ -221,16 +224,21 @@ function buildMiniflareOptions( ? STATIC_ASSET_EXTENSIONS.slice() : null; - async function logRequest(request: Request): Promise { - const durationMs = Number(request.headers.get('o2-duration-ms') || 0); - const responseStatus = Number( - request.headers.get('o2-response-status') || 200, - ); + const wrappedHook = requestHook + ? async (request: Request) => { + const durationMs = Number(request.headers.get('o2-duration-ms') || 0); + const responseStatus = Number( + request.headers.get('o2-response-status') || 200, + ); - logRequestLine?.(request, {responseStatus, durationMs}); + await requestHook(request, { + responseStatus, + durationMs, + }); - return new Response('ok'); - } + return new Response('ok'); + } + : null; const wrappedBindings = new Set( workers @@ -270,16 +278,16 @@ function buildMiniflareOptions( { name: ROUTING_WORKER_NAME, modules: true, - script: `export default { fetch: ${miniOxygenHandler.toString()} }`, + script: getMiniOxygenHandlerScript(), bindings: { staticAssetExtensions, oxygenHeadersMap, - }, + } satisfies OnlyBindings, serviceBindings: { entry: entryWorker.name, - logRequest, + ...(wrappedHook && {hook: wrappedHook}), ...(handleAssets && {assets: handleAssets}), - }, + } satisfies OnlyServices, }, ...workers.map((worker) => { const isNormalWorker = !wrappedBindings.has(worker.name); @@ -312,3 +320,22 @@ function createAssetHandler(options: Partial) { ); }; } + +// --- Utility types: +type OnlyServiceKeys = Exclude< + { + [P in keyof T]: NonNullable extends {fetch: Function} ? P : never; + }[keyof T], + undefined +>; + +type OnlyServices = Pick< + {[key in keyof T]: string | ((request: Request) => Promise)}, + OnlyServiceKeys +>; + +type UnionUndefinedToNull = T extends undefined ? null : T; +type OnlyBindings = Omit< + {[key in keyof T]: UnionUndefinedToNull}, + OnlyServiceKeys +>; From d24570942f306af3f6543a9829ecc7e194853162 Mon Sep 17 00:00:00 2001 From: Fran Dios Date: Fri, 5 Apr 2024 17:28:45 +0200 Subject: [PATCH 07/13] Replicate requestHook at the Vite Environment level --- packages/mini-oxygen/src/vite/plugin.ts | 1 + .../mini-oxygen/src/vite/server-middleware.ts | 45 +++++++++++++++---- packages/mini-oxygen/src/vite/worker-entry.ts | 12 ++++- packages/mini-oxygen/src/worker/handler.ts | 24 +++++++--- packages/mini-oxygen/src/worker/index.ts | 41 ++++------------- packages/mini-oxygen/src/worker/utils.ts | 28 ++++++++++++ 6 files changed, 103 insertions(+), 48 deletions(-) create mode 100644 packages/mini-oxygen/src/worker/utils.ts diff --git a/packages/mini-oxygen/src/vite/plugin.ts b/packages/mini-oxygen/src/vite/plugin.ts index d1e3ca5ad9..c104000482 100644 --- a/packages/mini-oxygen/src/vite/plugin.ts +++ b/packages/mini-oxygen/src/vite/plugin.ts @@ -105,6 +105,7 @@ export function oxygen(pluginOptions: OxygenPluginOptions = {}): Plugin[] { debug: apiOptions.debug ?? pluginOptions.debug ?? false, inspectorPort: apiOptions.inspectorPort ?? pluginOptions.inspectorPort, + requestHook: apiOptions.requestHook, logRequestLine: // Give priority to the plugin option over the CLI option here, // since the CLI one is just a default, not a user-provided flag. diff --git a/packages/mini-oxygen/src/vite/server-middleware.ts b/packages/mini-oxygen/src/vite/server-middleware.ts index c372afa1d4..e82aaf22e7 100644 --- a/packages/mini-oxygen/src/vite/server-middleware.ts +++ b/packages/mini-oxygen/src/vite/server-middleware.ts @@ -1,15 +1,28 @@ import {fetchModule, type ViteDevServer} from 'vite'; import {fileURLToPath} from 'node:url'; -import {fetch, createMiniOxygen, Request, Response} from '../worker/index.js'; +import { + fetch, + createMiniOxygen, + Request, + Response, + type RequestHook, + defaultLogRequestLine, +} from '../worker/index.js'; +import type {OnlyBindings, OnlyServices} from '../worker/utils.js'; import {getHmrUrl, pipeFromWeb, toURL, toWeb} from './utils.js'; import type {ViteEnv} from './worker-entry.js'; +import {getRequestInfo} from '../worker/handler.js'; const scriptPath = fileURLToPath(new URL('./worker-entry.js', import.meta.url)); const FETCH_MODULE_PATHNAME = '/__vite_fetch_module'; const WARMUP_PATHNAME = '/__vite_warmup'; export type InternalMiniOxygenOptions = { + /** + * A function called asynchronously when the worker gets a response. + */ + requestHook?: RequestHook; /** * Creates bindings in `env` that can be used to fetch external services. */ @@ -48,7 +61,7 @@ export type MiniOxygenViteOptions = InternalMiniOxygenOptions & { env?: {[key: string]: string}; debug?: boolean; inspectorPort?: number; - logRequestLine?: null | ((request: Request) => void); + logRequestLine?: null | RequestHook; }; export type MiniOxygen = Awaited>; @@ -59,9 +72,10 @@ export async function startMiniOxygenRuntime({ services, debug = false, inspectorPort, - logRequestLine, + logRequestLine = defaultLogRequestLine, crossBoundarySetup, entry: workerEntryFile, + requestHook, }: MiniOxygenViteOptions) { const serviceBindings = services && @@ -73,6 +87,20 @@ export async function startMiniOxygenRuntime({ ]), ); + const wrappedHook = + requestHook || logRequestLine + ? async (request: Request) => { + const info = getRequestInfo(request.headers); + + await Promise.all([ + requestHook?.(request, info), + logRequestLine?.(request, info), + ]); + + return new Response('ok'); + } + : null; + const miniOxygen = createMiniOxygen({ debug, inspectorPort, @@ -82,7 +110,10 @@ export async function startMiniOxygenRuntime({ name: 'vite-env', modulesRoot: '/', modules: [{type: 'ESModule', path: scriptPath}], - serviceBindings, + serviceBindings: { + ...serviceBindings, + ...(wrappedHook && {__VITE_REQUEST_HOOK: wrappedHook}), + } satisfies OnlyServices, bindings: { ...env, __VITE_ROOT: viteDevServer.config.root, @@ -90,11 +121,9 @@ export async function startMiniOxygenRuntime({ __VITE_FETCH_MODULE_PATHNAME: FETCH_MODULE_PATHNAME, __VITE_HMR_URL: getHmrUrl(viteDevServer), __VITE_WARMUP_PATHNAME: WARMUP_PATHNAME, - } satisfies Omit, + } satisfies OnlyBindings, unsafeEvalBinding: '__VITE_UNSAFE_EVAL', - wrappedBindings: { - __VITE_SETUP_ENV: 'setup-environment', - }, + wrappedBindings: {__VITE_SETUP_ENV: 'setup-environment'}, }, { name: 'setup-environment', diff --git a/packages/mini-oxygen/src/vite/worker-entry.ts b/packages/mini-oxygen/src/vite/worker-entry.ts index b4281480dc..c07748297a 100644 --- a/packages/mini-oxygen/src/vite/worker-entry.ts +++ b/packages/mini-oxygen/src/vite/worker-entry.ts @@ -13,6 +13,7 @@ import { } from 'vite/runtime'; import type {HMRPayload} from 'vite'; import type {Response} from 'miniflare'; +import {withRequestHook} from '../worker/handler.js'; export interface ViteEnv { __VITE_ROOT: string; @@ -20,6 +21,7 @@ export interface ViteEnv { __VITE_FETCH_MODULE_PATHNAME: string; __VITE_RUNTIME_EXECUTE_URL: string; __VITE_WARMUP_PATHNAME: string; + __VITE_REQUEST_HOOK?: {fetch: typeof fetch}; __VITE_SETUP_ENV: (request: Request) => void; // Ref: https://github.com/cloudflare/workerd/blob/main/src/workerd/api/unsafe.h __VITE_UNSAFE_EVAL: { @@ -35,7 +37,7 @@ export default { /** * Worker entry module that wraps the user app's entry module. */ - async fetch(request: Request, env: ViteEnv, ctx: any) { + async fetch(request: Request, env: ViteEnv, context: ExecutionContext) { env.__VITE_SETUP_ENV(request); const url = new URL(request.url); @@ -48,7 +50,13 @@ export default { } // Execute the user app's entry module. - return module.default.fetch(request, createUserEnv(env), ctx); + return withRequestHook({ + request, + context, + hook: env.__VITE_REQUEST_HOOK, + handleRequest: () => + module.default.fetch(request, createUserEnv(env), context), + }); }, }; diff --git a/packages/mini-oxygen/src/worker/handler.ts b/packages/mini-oxygen/src/worker/handler.ts index 994d44462f..0943d1dddf 100644 --- a/packages/mini-oxygen/src/worker/handler.ts +++ b/packages/mini-oxygen/src/worker/handler.ts @@ -48,31 +48,36 @@ async function miniOxygenHandler( }, } satisfies RequestInit; + const handleRequest = () => env.entry.fetch(request, requestInit); + return env.hook ? withRequestHook({ + ...requestInit, + handleRequest, request: new Request(request, requestInit), - handler: env.entry, hook: env.hook, context, }) - : env.entry.fetch(request, requestInit); + : handleRequest(); } type RequestHookOptions = { - handler: Service; + handleRequest: () => Response | Promise; request: Request; + headers?: Record; context: ExecutionContext; hook?: Service; }; export async function withRequestHook({ + handleRequest, request, - handler, + headers, hook, context, }: RequestHookOptions) { const startTimeMs = Date.now(); - const response = await handler.fetch(request); + const response = await handleRequest(); const durationMs = Date.now() - startTimeMs; if (hook) { @@ -81,7 +86,7 @@ export async function withRequestHook({ method: request.method, signal: request.signal, headers: { - ...Object.fromEntries(request.headers.entries()), + ...headers, 'o2-duration-ms': String(durationMs), 'o2-response-status': String(response.status), }, @@ -91,3 +96,10 @@ export async function withRequestHook({ return response; } + +export function getRequestInfo(headers: {get(name: string): string | null}) { + return { + durationMs: Number(headers.get('o2-duration-ms') || 0), + responseStatus: Number(headers.get('o2-response-status') || 200), + }; +} diff --git a/packages/mini-oxygen/src/worker/index.ts b/packages/mini-oxygen/src/worker/index.ts index 2ad61f1043..5f9adba471 100644 --- a/packages/mini-oxygen/src/worker/index.ts +++ b/packages/mini-oxygen/src/worker/index.ts @@ -18,12 +18,14 @@ import { } from './assets.js'; import { getMiniOxygenHandlerScript, + getRequestInfo, type MiniOxygenHandlerEnv, } from './handler.js'; import {OXYGEN_HEADERS_MAP} from '../common/headers.js'; import {findPort} from '../common/find-port.js'; import {OXYGEN_COMPAT_PARAMS} from '../common/compat.js'; import {isO2Verbose} from '../common/debug.js'; +import type {OnlyBindings, OnlyServices} from './utils.js'; export { buildAssetsUrl, @@ -172,12 +174,15 @@ export function createMiniOxygen({ }; } -type RequestHook = ( +export type RequestHook = ( request: Request, opt: {responseStatus: number; durationMs: number}, ) => void | Promise; -const defaultRequestHook: RequestHook = (request, {responseStatus}) => { +export const defaultLogRequestLine: RequestHook = ( + request, + {responseStatus}, +) => { console.log( `${request.method} ${responseStatus} ${request.url.replace( new URL(request.url).origin, @@ -211,7 +216,7 @@ const UNSAFE_OUTBOUND_SERVICE = { function buildMiniflareOptions( {workers, ...mfOverwriteOptions}: InputMiniflareOptions, - requestHook: RequestHook | null = defaultRequestHook, + requestHook: RequestHook | null = defaultLogRequestLine, assetsOptions?: AssetOptions, ): OutputMiniflareOptions { const entryWorker = workers.find((worker) => !!worker.name); @@ -226,16 +231,7 @@ function buildMiniflareOptions( const wrappedHook = requestHook ? async (request: Request) => { - const durationMs = Number(request.headers.get('o2-duration-ms') || 0); - const responseStatus = Number( - request.headers.get('o2-response-status') || 200, - ); - - await requestHook(request, { - responseStatus, - durationMs, - }); - + await requestHook(request, getRequestInfo(request.headers)); return new Response('ok'); } : null; @@ -320,22 +316,3 @@ function createAssetHandler(options: Partial) { ); }; } - -// --- Utility types: -type OnlyServiceKeys = Exclude< - { - [P in keyof T]: NonNullable extends {fetch: Function} ? P : never; - }[keyof T], - undefined ->; - -type OnlyServices = Pick< - {[key in keyof T]: string | ((request: Request) => Promise)}, - OnlyServiceKeys ->; - -type UnionUndefinedToNull = T extends undefined ? null : T; -type OnlyBindings = Omit< - {[key in keyof T]: UnionUndefinedToNull}, - OnlyServiceKeys ->; diff --git a/packages/mini-oxygen/src/worker/utils.ts b/packages/mini-oxygen/src/worker/utils.ts new file mode 100644 index 0000000000..992d27ed8d --- /dev/null +++ b/packages/mini-oxygen/src/worker/utils.ts @@ -0,0 +1,28 @@ +import type {Request} from 'miniflare'; +// --- Utility types: +type OnlyServiceKeys = Exclude< + { + [P in keyof T]: NonNullable extends {fetch: Function} ? P : never; + }[keyof T], + undefined +>; + +type OtherKeys = Exclude< + { + [P in keyof T]: NonNullable extends Function | {eval: Function} + ? P + : never; + }[keyof T], + undefined +>; + +export type OnlyServices = Pick< + {[key in keyof T]: string | ((request: Request) => Promise)}, + OnlyServiceKeys +>; + +type UnionUndefinedToNull = T extends undefined ? null : T; +export type OnlyBindings = Omit< + {[key in keyof T]: UnionUndefinedToNull}, + OnlyServiceKeys | OtherKeys +>; From 8cea5838b277f51c8a9ff116306db72d870f4b67 Mon Sep 17 00:00:00 2001 From: Fran Dios Date: Fri, 5 Apr 2024 17:36:06 +0200 Subject: [PATCH 08/13] Fix re-using request body --- packages/mini-oxygen/src/vite/worker-entry.ts | 18 +++++++----- packages/mini-oxygen/src/worker/handler.ts | 29 +++++++++---------- 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/packages/mini-oxygen/src/vite/worker-entry.ts b/packages/mini-oxygen/src/vite/worker-entry.ts index c07748297a..a85965b50a 100644 --- a/packages/mini-oxygen/src/vite/worker-entry.ts +++ b/packages/mini-oxygen/src/vite/worker-entry.ts @@ -49,14 +49,18 @@ export default { return new globalThis.Response(null); } + const handleRequest = () => + module.default.fetch(request, createUserEnv(env), context); + // Execute the user app's entry module. - return withRequestHook({ - request, - context, - hook: env.__VITE_REQUEST_HOOK, - handleRequest: () => - module.default.fetch(request, createUserEnv(env), context), - }); + return env.__VITE_REQUEST_HOOK + ? withRequestHook({ + request, + context, + hook: env.__VITE_REQUEST_HOOK, + handleRequest, + }) + : handleRequest(); }, }; diff --git a/packages/mini-oxygen/src/worker/handler.ts b/packages/mini-oxygen/src/worker/handler.ts index 0943d1dddf..ebde4ccb87 100644 --- a/packages/mini-oxygen/src/worker/handler.ts +++ b/packages/mini-oxygen/src/worker/handler.ts @@ -54,7 +54,8 @@ async function miniOxygenHandler( ? withRequestHook({ ...requestInit, handleRequest, - request: new Request(request, requestInit), + request, + headers: requestInit.headers, hook: env.hook, context, }) @@ -66,7 +67,7 @@ type RequestHookOptions = { request: Request; headers?: Record; context: ExecutionContext; - hook?: Service; + hook: Service; }; export async function withRequestHook({ @@ -80,19 +81,17 @@ export async function withRequestHook({ const response = await handleRequest(); const durationMs = Date.now() - startTimeMs; - if (hook) { - context.waitUntil( - hook.fetch(request.url, { - method: request.method, - signal: request.signal, - headers: { - ...headers, - 'o2-duration-ms': String(durationMs), - 'o2-response-status': String(response.status), - }, - }), - ); - } + context.waitUntil( + hook.fetch(request.url, { + method: request.method, + signal: request.signal, + headers: { + ...headers, + 'o2-duration-ms': String(durationMs), + 'o2-response-status': String(response.status), + }, + }), + ); return response; } From 5bacd31d666f0cbce6fa1ee0fd5146b77327a268 Mon Sep 17 00:00:00 2001 From: Fran Dios Date: Fri, 5 Apr 2024 18:32:11 +0200 Subject: [PATCH 09/13] Make the request hook be a POST request --- packages/cli/src/lib/mini-oxygen/common.ts | 30 ++++----- packages/cli/src/lib/mini-oxygen/node.ts | 21 ++++++- .../mini-oxygen/src/vite/server-middleware.ts | 11 ++-- packages/mini-oxygen/src/worker/handler.ts | 61 +++++++++++++------ packages/mini-oxygen/src/worker/index.ts | 17 ++---- 5 files changed, 85 insertions(+), 55 deletions(-) diff --git a/packages/cli/src/lib/mini-oxygen/common.ts b/packages/cli/src/lib/mini-oxygen/common.ts index 05781b8766..3736cf2a7a 100644 --- a/packages/cli/src/lib/mini-oxygen/common.ts +++ b/packages/cli/src/lib/mini-oxygen/common.ts @@ -6,6 +6,7 @@ import { import colors from '@shopify/cli-kit/node/colors'; import {DEV_ROUTES} from '../request-events.js'; import {AbortError} from '@shopify/cli-kit/node/error'; +import type {RequestHookInfo} from '@shopify/mini-oxygen'; // Default port used for debugging in VSCode and Chrome DevTools. export const DEFAULT_INSPECTOR_PORT = 9229; @@ -19,16 +20,11 @@ export function handleMiniOxygenImportFail(): never { ); } -export function logRequestLine( - // Minimal overlap between Fetch, Miniflare@2 and Miniflare@3 request types. - request: Pick & { - headers: {get: (key: string) => string | null}; - }, - { - responseStatus = 200, - durationMs = 0, - }: {responseStatus?: number; durationMs?: number} = {}, -): void { +export function logRequestLine({ + request, + response, + meta, +}: RequestHookInfo): void { try { const url = new URL(request.url); if (DEV_ROUTES.has(url.pathname) || url.pathname === '/favicon.ico') return; @@ -46,26 +42,26 @@ export function logRequestLine( } const colorizeStatus = - responseStatus < 300 + response.status < 300 ? outputToken.green - : responseStatus < 400 + : response.status < 400 ? outputToken.cyan : outputToken.errorText; outputInfo( outputContent`${request.method.padStart(6)} ${colorizeStatus( - String(responseStatus), + String(response.status), )} ${outputToken.italic(type.padEnd(7, ' '))} ${route} ${ - durationMs > 0 ? colors.dim(` ${durationMs}ms`) : '' + meta.durationMs > 0 ? colors.dim(` ${meta.durationMs}ms`) : '' }${info ? ' ' + colors.dim(info) : ''}${ - request.headers.get('purpose') === 'prefetch' + request.headers['purpose'] === 'prefetch' ? outputToken.italic(colors.dim(' prefetch')) : '' }`, ); } catch { - if (request && responseStatus) { - outputInfo(`${request.method} ${responseStatus} ${request.url}`); + if (request && response?.status) { + outputInfo(`${request.method} ${response.status} ${request.url}`); } } } diff --git a/packages/cli/src/lib/mini-oxygen/node.ts b/packages/cli/src/lib/mini-oxygen/node.ts index b751aea1cc..5753537a99 100644 --- a/packages/cli/src/lib/mini-oxygen/node.ts +++ b/packages/cli/src/lib/mini-oxygen/node.ts @@ -87,9 +87,24 @@ export async function startNodeServer({ () => defaultDispatcher(request), ); - logRequestLine(request, { - responseStatus: response.status, - durationMs: startTimeMs > 0 ? Date.now() - startTimeMs : 0, + const endTimeMs = Date.now(); + + logRequestLine({ + request: { + url: request.url, + method: request.method, + headers: Object.fromEntries(request.headers.entries()), + }, + response: { + status: response.status, + statusText: response.statusText, + headers: Object.fromEntries(response.headers.entries()), + }, + meta: { + startTimeMs, + endTimeMs, + durationMs: startTimeMs > 0 ? endTimeMs - startTimeMs : 0, + }, }); return response; diff --git a/packages/mini-oxygen/src/vite/server-middleware.ts b/packages/mini-oxygen/src/vite/server-middleware.ts index e82aaf22e7..9ec0bb5d6d 100644 --- a/packages/mini-oxygen/src/vite/server-middleware.ts +++ b/packages/mini-oxygen/src/vite/server-middleware.ts @@ -12,7 +12,7 @@ import type {OnlyBindings, OnlyServices} from '../worker/utils.js'; import {getHmrUrl, pipeFromWeb, toURL, toWeb} from './utils.js'; import type {ViteEnv} from './worker-entry.js'; -import {getRequestInfo} from '../worker/handler.js'; +import {type RequestHookInfo} from '../worker/handler.js'; const scriptPath = fileURLToPath(new URL('./worker-entry.js', import.meta.url)); const FETCH_MODULE_PATHNAME = '/__vite_fetch_module'; @@ -72,10 +72,10 @@ export async function startMiniOxygenRuntime({ services, debug = false, inspectorPort, - logRequestLine = defaultLogRequestLine, crossBoundarySetup, entry: workerEntryFile, requestHook, + logRequestLine = defaultLogRequestLine, }: MiniOxygenViteOptions) { const serviceBindings = services && @@ -90,12 +90,9 @@ export async function startMiniOxygenRuntime({ const wrappedHook = requestHook || logRequestLine ? async (request: Request) => { - const info = getRequestInfo(request.headers); + const info = (await request.json()) as RequestHookInfo; - await Promise.all([ - requestHook?.(request, info), - logRequestLine?.(request, info), - ]); + await Promise.all([requestHook?.(info), logRequestLine?.(info)]); return new Response('ok'); } diff --git a/packages/mini-oxygen/src/worker/handler.ts b/packages/mini-oxygen/src/worker/handler.ts index ebde4ccb87..f9d07aa8e6 100644 --- a/packages/mini-oxygen/src/worker/handler.ts +++ b/packages/mini-oxygen/src/worker/handler.ts @@ -1,5 +1,3 @@ -// This file is stringified, do not import anything here. - type Service = {fetch: typeof fetch}; export type MiniOxygenHandlerEnv = { @@ -14,6 +12,7 @@ export function getMiniOxygenHandlerScript() { return `export default { fetch: ${miniOxygenHandler} }\n${withRequestHook}`; } +// This function is stringified, do not use anything from outer scope here: async function miniOxygenHandler( request: Request, env: MiniOxygenHandlerEnv, @@ -70,35 +69,63 @@ type RequestHookOptions = { hook: Service; }; +/** + * @public + */ +export type RequestHookInfo = { + request: { + url: string; + method: string; + headers: Record; + }; + response: { + status: number; + statusText: string; + headers: Record; + }; + meta: { + startTimeMs: number; + endTimeMs: number; + durationMs: number; + }; +}; + +// This function is stringified, do not use anything from outer scope here: export async function withRequestHook({ handleRequest, request, - headers, + headers = {}, hook, context, }: RequestHookOptions) { const startTimeMs = Date.now(); const response = await handleRequest(); - const durationMs = Date.now() - startTimeMs; + const endTimeMs = Date.now(); + const durationMs = endTimeMs - startTimeMs; context.waitUntil( hook.fetch(request.url, { - method: request.method, + method: 'POST', signal: request.signal, - headers: { - ...headers, - 'o2-duration-ms': String(durationMs), - 'o2-response-status': String(response.status), - }, + body: JSON.stringify({ + request: { + url: request.url, + method: request.method, + headers, + }, + response: { + status: response.status, + statusText: response.statusText, + headers: Object.fromEntries(response.headers.entries()), + }, + meta: { + startTimeMs, + endTimeMs, + durationMs, + }, + } satisfies RequestHookInfo), }), ); return response; } - -export function getRequestInfo(headers: {get(name: string): string | null}) { - return { - durationMs: Number(headers.get('o2-duration-ms') || 0), - responseStatus: Number(headers.get('o2-response-status') || 200), - }; -} diff --git a/packages/mini-oxygen/src/worker/index.ts b/packages/mini-oxygen/src/worker/index.ts index 5f9adba471..05438897d5 100644 --- a/packages/mini-oxygen/src/worker/index.ts +++ b/packages/mini-oxygen/src/worker/index.ts @@ -18,8 +18,8 @@ import { } from './assets.js'; import { getMiniOxygenHandlerScript, - getRequestInfo, type MiniOxygenHandlerEnv, + type RequestHookInfo, } from './handler.js'; import {OXYGEN_HEADERS_MAP} from '../common/headers.js'; import {findPort} from '../common/find-port.js'; @@ -34,6 +34,7 @@ export { fetch, type RequestInit, type ResponseInit, + type RequestHookInfo, }; const DEFAULT_PUBLIC_INSPECTOR_PORT = 9229; @@ -174,17 +175,11 @@ export function createMiniOxygen({ }; } -export type RequestHook = ( - request: Request, - opt: {responseStatus: number; durationMs: number}, -) => void | Promise; +export type RequestHook = (info: RequestHookInfo) => void | Promise; -export const defaultLogRequestLine: RequestHook = ( - request, - {responseStatus}, -) => { +export const defaultLogRequestLine: RequestHook = ({request, response}) => { console.log( - `${request.method} ${responseStatus} ${request.url.replace( + `${request.method} ${response.status} ${request.url.replace( new URL(request.url).origin, '', )}`, @@ -231,7 +226,7 @@ function buildMiniflareOptions( const wrappedHook = requestHook ? async (request: Request) => { - await requestHook(request, getRequestInfo(request.headers)); + await requestHook((await request.json()) as RequestHookInfo); return new Response('ok'); } : null; From e075be5cf4431fa361e7f3374fb1bddd973b2388 Mon Sep 17 00:00:00 2001 From: Fran Dios Date: Fri, 5 Apr 2024 18:53:40 +0200 Subject: [PATCH 10/13] Move source of truth for sub-request profiler types to Hydrogen package --- packages/hydrogen/src/cache/fetch.ts | 2 -- packages/hydrogen/src/hydrogen.d.ts | 8 +++++++ packages/hydrogen/src/vite/plugin.ts | 9 +------- packages/remix-oxygen/src/event-logger.ts | 27 +---------------------- packages/remix-oxygen/src/server.ts | 7 ++---- 5 files changed, 12 insertions(+), 41 deletions(-) diff --git a/packages/hydrogen/src/cache/fetch.ts b/packages/hydrogen/src/cache/fetch.ts index 26911bbbbe..d9f7880ed4 100644 --- a/packages/hydrogen/src/cache/fetch.ts +++ b/packages/hydrogen/src/cache/fetch.ts @@ -1,5 +1,3 @@ -/// - import {hashKey} from '../utils/hash.js'; import { CacheShort, diff --git a/packages/hydrogen/src/hydrogen.d.ts b/packages/hydrogen/src/hydrogen.d.ts index 686ba927e7..964b0028d5 100644 --- a/packages/hydrogen/src/hydrogen.d.ts +++ b/packages/hydrogen/src/hydrogen.d.ts @@ -4,6 +4,7 @@ import type { SessionData, FlashSessionData, } from '@remix-run/server-runtime'; +import type {RequestEventPayload} from './vite/request-events'; export interface HydrogenSessionData { customerAccount: { @@ -29,3 +30,10 @@ export interface HydrogenSession< SessionStorage['commitSession'] >; } + +declare global { + var __H2O_LOG_EVENT: undefined | ((event: RequestEventPayload) => void); + var __remix_devServerHooks: + | undefined + | {getCriticalCss: (...args: unknown[]) => any}; +} diff --git a/packages/hydrogen/src/vite/plugin.ts b/packages/hydrogen/src/vite/plugin.ts index 6c1827a9d2..d251316b76 100644 --- a/packages/hydrogen/src/vite/plugin.ts +++ b/packages/hydrogen/src/vite/plugin.ts @@ -4,17 +4,10 @@ import type {HydrogenPluginOptions} from './types.js'; // @ts-ignore -- Module outside of the rootDir import type {OxygenApiOptions} from '~/mini-oxygen/vite/plugin.js'; -import {type RequestEventPayload, emitRequestEvent} from './request-events.js'; +import {emitRequestEvent} from './request-events.js'; export type {HydrogenPluginOptions}; -declare global { - var __H2O_LOG_EVENT: undefined | ((event: RequestEventPayload) => void); - var __remix_devServerHooks: - | undefined - | {getCriticalCss: (...args: unknown[]) => any}; -} - /** * Enables Hydrogen utilities for local development * such as GraphiQL, Subrequest Profiler, etc. diff --git a/packages/remix-oxygen/src/event-logger.ts b/packages/remix-oxygen/src/event-logger.ts index 99a177891b..0ba62eec65 100644 --- a/packages/remix-oxygen/src/event-logger.ts +++ b/packages/remix-oxygen/src/event-logger.ts @@ -1,29 +1,4 @@ -// Make sure to match this type with the one in packages/cli/src/lib/request-events.ts -export type H2OEvent = { - url: string; - eventType: 'request' | 'subrequest'; - requestId?: string | null; - purpose?: string | null; - startTime: number; - endTime?: number; - cacheStatus?: 'MISS' | 'HIT' | 'STALE' | 'PUT'; - waitUntil?: ExecutionContext['waitUntil']; - graphql?: string | null; - stackInfo?: { - file?: string; - func?: string; - line?: number; - column?: number; - }; - responsePayload?: any; - responseInit?: ResponseInit; - cache?: { - status?: string; - strategy?: string; - key?: string | readonly unknown[]; - }; - displayName?: string; -}; +type H2OEvent = Parameters>[0]; let hasWarned = false; diff --git a/packages/remix-oxygen/src/server.ts b/packages/remix-oxygen/src/server.ts index 4a4b87d5da..dc22819472 100644 --- a/packages/remix-oxygen/src/server.ts +++ b/packages/remix-oxygen/src/server.ts @@ -1,13 +1,10 @@ +/// import { createRequestHandler as createRemixRequestHandler, type AppLoadContext, type ServerBuild, } from '@remix-run/server-runtime'; -import {createEventLogger, type H2OEvent} from './event-logger'; - -declare global { - var __H2O_LOG_EVENT: undefined | ((event: H2OEvent) => void); -} +import {createEventLogger} from './event-logger'; const originalErrorToString = Error.prototype.toString; Error.prototype.toString = function () { From 10f5c0ebc5f6ae8e2d74ff3eb3405e887b425054 Mon Sep 17 00:00:00 2001 From: Fran Dios Date: Fri, 5 Apr 2024 19:11:25 +0200 Subject: [PATCH 11/13] Fix cross-package types --- packages/cli/src/commands/hydrogen/dev-vite.ts | 4 ++-- packages/cli/tsconfig.json | 7 +++++-- packages/hydrogen/src/vite/plugin.ts | 4 ++-- packages/mini-oxygen/src/worker/index.ts | 12 +++++++----- packages/mini-oxygen/tsconfig.json | 1 - turbo.json | 8 ++++---- 6 files changed, 20 insertions(+), 16 deletions(-) diff --git a/packages/cli/src/commands/hydrogen/dev-vite.ts b/packages/cli/src/commands/hydrogen/dev-vite.ts index 7bdcc6c9d1..2488d7ea31 100644 --- a/packages/cli/src/commands/hydrogen/dev-vite.ts +++ b/packages/cli/src/commands/hydrogen/dev-vite.ts @@ -37,8 +37,8 @@ import {findPort} from '../../lib/find-port.js'; import {logRequestLine} from '../../lib/mini-oxygen/common.js'; // Do not import JS from here, only types -import type {OxygenApiOptions} from '@shopify/mini-oxygen/vite'; -import type {HydrogenPluginOptions} from '@shopify/hydrogen/vite'; +import type {OxygenApiOptions} from '~/mini-oxygen/vite/plugin.js'; +import type {HydrogenPluginOptions} from '~/hydrogen/vite/plugin.js'; export default class DevVite extends Command { static description = diff --git a/packages/cli/tsconfig.json b/packages/cli/tsconfig.json index b2a0812875..e6f87be962 100644 --- a/packages/cli/tsconfig.json +++ b/packages/cli/tsconfig.json @@ -6,9 +6,12 @@ "module": "NodeNext", "moduleResolution": "NodeNext", "jsx": "react-jsx", - "rootDir": "src", "noUncheckedIndexedAccess": true, "types": ["@shopify/oxygen-workers-types", "node", "@remix-run/dev"], - "outDir": "dist" + "outDir": "dist", + "paths": { + "~/mini-oxygen/*": ["../mini-oxygen/src/*"], + "~/hydrogen/*": ["../hydrogen/src/*"] + } } } diff --git a/packages/hydrogen/src/vite/plugin.ts b/packages/hydrogen/src/vite/plugin.ts index d251316b76..f5d13f84fc 100644 --- a/packages/hydrogen/src/vite/plugin.ts +++ b/packages/hydrogen/src/vite/plugin.ts @@ -1,10 +1,10 @@ import type {Plugin, ResolvedConfig} from 'vite'; import {setupHydrogenMiddleware} from './hydrogen-middleware.js'; import type {HydrogenPluginOptions} from './types.js'; +import {emitRequestEvent} from './request-events.js'; -// @ts-ignore -- Module outside of the rootDir +// Do not import JS from here, only types import type {OxygenApiOptions} from '~/mini-oxygen/vite/plugin.js'; -import {emitRequestEvent} from './request-events.js'; export type {HydrogenPluginOptions}; diff --git a/packages/mini-oxygen/src/worker/index.ts b/packages/mini-oxygen/src/worker/index.ts index 05438897d5..ed950a15f9 100644 --- a/packages/mini-oxygen/src/worker/index.ts +++ b/packages/mini-oxygen/src/worker/index.ts @@ -97,11 +97,13 @@ export function createMiniOxygen({ )) || miniflareOptions.workers[0]; - if ('scriptPath' in mainWorker) { - sourceMapPath = mainWorker.scriptPath + '.map'; - } else if (Array.isArray(mainWorker?.modules)) { - const modulePath = mainWorker?.modules[0]!.path; - sourceMapPath = modulePath + '.map'; + if (mainWorker) { + if ('scriptPath' in mainWorker) { + sourceMapPath = mainWorker.scriptPath + '.map'; + } else if (Array.isArray(mainWorker?.modules)) { + const modulePath = mainWorker?.modules[0]!.path; + sourceMapPath = modulePath + '.map'; + } } } diff --git a/packages/mini-oxygen/tsconfig.json b/packages/mini-oxygen/tsconfig.json index c431c7e4ed..2c9211a975 100644 --- a/packages/mini-oxygen/tsconfig.json +++ b/packages/mini-oxygen/tsconfig.json @@ -3,7 +3,6 @@ "include": ["src"], "exclude": ["dist", "tests", "node_modules", ".turbo"], "compilerOptions": { - "rootDir": "src", "target": "ES2022", "outDir": "dist", "module": "NodeNext", diff --git a/turbo.json b/turbo.json index e2a16e03cb..bed6c8ffa6 100644 --- a/turbo.json +++ b/turbo.json @@ -39,11 +39,11 @@ "@shopify/create-hydrogen#build": { "dependsOn": ["@shopify/cli-hydrogen#build"] }, + "@shopify/remix-oxygen#build": { + "dependsOn": ["@shopify/hydrogen#build"] + }, "@shopify/hydrogen#build": { - "dependsOn": [ - "@shopify/hydrogen-react#build", - "@shopify/remix-oxygen#build" - ] + "dependsOn": ["@shopify/hydrogen-react#build"] }, "hello-world#build": { "dependsOn": [ From c77093d68c799717a0a57ad59f8b51b353f1f9b6 Mon Sep 17 00:00:00 2001 From: Fran Dios Date: Fri, 5 Apr 2024 19:27:23 +0200 Subject: [PATCH 12/13] Start sending subrequest-profiler events from Hydrogen middleware instead of remix-oxygen --- packages/hydrogen/src/vite/plugin.ts | 42 +++++++++++++++----- packages/hydrogen/src/vite/request-events.ts | 20 +++++++--- packages/mini-oxygen/src/worker/handler.ts | 4 +- 3 files changed, 49 insertions(+), 17 deletions(-) diff --git a/packages/hydrogen/src/vite/plugin.ts b/packages/hydrogen/src/vite/plugin.ts index f5d13f84fc..9589c64d52 100644 --- a/packages/hydrogen/src/vite/plugin.ts +++ b/packages/hydrogen/src/vite/plugin.ts @@ -1,7 +1,7 @@ import type {Plugin, ResolvedConfig} from 'vite'; import {setupHydrogenMiddleware} from './hydrogen-middleware.js'; import type {HydrogenPluginOptions} from './types.js'; -import {emitRequestEvent} from './request-events.js'; +import {type RequestEventPayload, emitRequestEvent} from './request-events.js'; // Do not import JS from here, only types import type {OxygenApiOptions} from '~/mini-oxygen/vite/plugin.js'; @@ -62,7 +62,39 @@ export function hydrogen(pluginOptions: HydrogenPluginOptions = {}): Plugin[] { if (oxygenPlugin) { oxygenPlugin.api?.registerPluginOptions?.({ shouldStartRuntime: () => !isRemixChildCompiler(resolvedConfig), + requestHook: ({request, response, meta}) => { + // Emit events for requests + emitRequestEvent( + { + __fromVite: true, + eventType: 'request', + url: request.url, + requestId: request.headers['request-id'], + purpose: request.headers['purpose'], + startTime: meta.startTimeMs, + responseInit: { + status: response.status, + statusText: response.statusText, + headers: Object.entries(response.headers), + }, + }, + resolvedConfig.root, + ); + }, crossBoundarySetup: [ + { + // Setup the global function in the Oxygen worker + script: (binding) => { + globalThis.__H2O_LOG_EVENT = binding; + }, + binding: (data) => { + // Emit events for subrequests from the parent process + emitRequestEvent( + data as RequestEventPayload, + resolvedConfig.root, + ); + }, + }, /** * To avoid initial CSS flash during development, * most frameworks implement a way to gather critical CSS. @@ -88,14 +120,6 @@ export function hydrogen(pluginOptions: HydrogenPluginOptions = {}): Plugin[] { ); }, }, - { - script: (binding) => { - globalThis.__H2O_LOG_EVENT = binding; - }, - binding: (data) => { - emitRequestEvent(data, resolvedConfig.root); - }, - }, ], } satisfies OxygenApiOptions); } else { diff --git a/packages/hydrogen/src/vite/request-events.ts b/packages/hydrogen/src/vite/request-events.ts index eb25110364..3c343227cd 100644 --- a/packages/hydrogen/src/vite/request-events.ts +++ b/packages/hydrogen/src/vite/request-events.ts @@ -16,6 +16,7 @@ const EVENT_MAP: Record = { }; export type RequestEventPayload = { + __fromVite?: boolean; url: string; eventType: 'request' | 'subrequest'; requestId?: string | null; @@ -32,7 +33,7 @@ export type RequestEventPayload = { column?: number; }; responsePayload?: any; - responseInit?: ResponseInit; + responseInit?: Omit & {headers?: [string, string][]}; cache?: { status?: string; strategy?: string; @@ -62,14 +63,21 @@ function getEventInfo(data: RequestEventPayload) { const eventEmitter = new EventEmitter(); const eventHistory: RequestEvent[] = []; -export function emitRequestEvent(payload: unknown, root: string) { - const maybeEvent = payload as RequestEventPayload; +export function emitRequestEvent(payload: RequestEventPayload, root: string) { + if (!payload || !payload.url || !payload.requestId) { + // Ignore incorrect events, although this should not happen. + return; + } - if (!maybeEvent || !('url' in maybeEvent) || !('requestId' in maybeEvent)) { + if (payload.eventType === 'request' && !payload.__fromVite) { + // Filter out events that come from @shopify/remix-oxygen, + // which is a deprecated way to send events. return; } - const {pathname} = new URL(maybeEvent.url, 'http://localhost'); + delete payload.__fromVite; + + const {pathname} = new URL(payload.url, 'http://localhost'); if (DEV_ROUTES.has(pathname)) return; const { @@ -80,7 +88,7 @@ export function emitRequestEvent(payload: unknown, root: string) { graphql, stackInfo, ...data - } = getEventInfo(maybeEvent); + } = getEventInfo(payload); let graphiqlLink = ''; let displayName = displayNameData; diff --git a/packages/mini-oxygen/src/worker/handler.ts b/packages/mini-oxygen/src/worker/handler.ts index f9d07aa8e6..9d6db740d2 100644 --- a/packages/mini-oxygen/src/worker/handler.ts +++ b/packages/mini-oxygen/src/worker/handler.ts @@ -94,7 +94,7 @@ export type RequestHookInfo = { export async function withRequestHook({ handleRequest, request, - headers = {}, + headers, hook, context, }: RequestHookOptions) { @@ -111,7 +111,7 @@ export async function withRequestHook({ request: { url: request.url, method: request.method, - headers, + headers: headers ?? Object.fromEntries(request.headers.entries()), }, response: { status: response.status, From b83a485ba4cfb73c48335aba64d8fc3c40c1f51f Mon Sep 17 00:00:00 2001 From: Fran Dios Date: Fri, 5 Apr 2024 19:59:41 +0200 Subject: [PATCH 13/13] Support subrequest profiler in Node.js servers --- .../hydrogen/src/vite/hydrogen-middleware.ts | 62 +++++++- packages/hydrogen/src/vite/plugin.ts | 140 +++++++++--------- packages/hydrogen/src/vite/request-events.ts | 6 +- 3 files changed, 134 insertions(+), 74 deletions(-) diff --git a/packages/hydrogen/src/vite/hydrogen-middleware.ts b/packages/hydrogen/src/vite/hydrogen-middleware.ts index 662008267f..2ea09d9e46 100644 --- a/packages/hydrogen/src/vite/hydrogen-middleware.ts +++ b/packages/hydrogen/src/vite/hydrogen-middleware.ts @@ -1,18 +1,76 @@ import {normalizePath, type ViteDevServer, type ResolvedConfig} from 'vite'; import path from 'node:path'; +import crypto from 'node:crypto'; import {createRequire} from 'node:module'; import {createReadStream} from 'node:fs'; -import {clearHistory, streamRequestEvents} from './request-events.js'; +import { + clearHistory, + emitRequestEvent, + streamRequestEvents, +} from './request-events.js'; import type {RemixPluginContext} from '@remix-run/dev/dist/vite/plugin.js'; import {addVirtualRoutes} from './add-virtual-routes.js'; import type {HydrogenPluginOptions} from './types.js'; const H2_PREFIX_WARN = '[h2:warn:vite] '; +export type HydrogenMiddlewareOptions = HydrogenPluginOptions & { + isOxygen?: boolean; +}; + export function setupHydrogenMiddleware( viteDevServer: ViteDevServer, - options: HydrogenPluginOptions, + options: HydrogenMiddlewareOptions, ) { + if (!options.isOxygen) { + // If Oxygen is not present, we are probably running + // on Node.js, so we can setup the global functions directly. + globalThis.__H2O_LOG_EVENT = (data) => { + emitRequestEvent(data, viteDevServer.config.root); + }; + + viteDevServer.middlewares.use(function (req, res, next) { + // Filter out dev requests + if (!/^\/__vite_/.test(req.url || '')) { + // Hydrogen requires a unique request ID for each request + // to track the request lifecycle. This is added by Oxygen + // normally but we can add it here for development in Node. + req.headers['request-id'] ??= crypto.randomUUID(); + + const startTimeMs = Date.now(); + let endTimeMs = 0; + + res.once('pipe', () => { + endTimeMs = Date.now(); + }); + + res.once('close', () => { + emitRequestEvent( + { + __fromVite: true, + eventType: 'request', + url: req.url!, + requestId: req.headers['request-id'] as string, + purpose: req.headers['purpose'] as string, + startTime: startTimeMs, + endTime: endTimeMs || Date.now(), + responseInit: { + status: res.statusCode, + statusText: res.statusMessage, + headers: Object.entries( + res.getHeaders() as Record, + ), + }, + }, + viteDevServer.config.root, + ); + }); + } + + next(); + }); + } + if (options.disableVirtualRoutes) return; addVirtualRoutesToRemix(viteDevServer); diff --git a/packages/hydrogen/src/vite/plugin.ts b/packages/hydrogen/src/vite/plugin.ts index 9589c64d52..d17a8c6e52 100644 --- a/packages/hydrogen/src/vite/plugin.ts +++ b/packages/hydrogen/src/vite/plugin.ts @@ -1,5 +1,8 @@ import type {Plugin, ResolvedConfig} from 'vite'; -import {setupHydrogenMiddleware} from './hydrogen-middleware.js'; +import { + HydrogenMiddlewareOptions, + setupHydrogenMiddleware, +} from './hydrogen-middleware.js'; import type {HydrogenPluginOptions} from './types.js'; import {type RequestEventPayload, emitRequestEvent} from './request-events.js'; @@ -15,7 +18,7 @@ export type {HydrogenPluginOptions}; * @experimental */ export function hydrogen(pluginOptions: HydrogenPluginOptions = {}): Plugin[] { - let apiOptions: HydrogenPluginOptions = {}; + let middlewareOptions: HydrogenMiddlewareOptions = {}; const isRemixChildCompiler = (config: ResolvedConfig) => !config.plugins?.some((plugin) => plugin.name === 'remix'); @@ -47,10 +50,10 @@ export function hydrogen(pluginOptions: HydrogenPluginOptions = {}): Plugin[] { }, api: { registerPluginOptions(newOptions: HydrogenPluginOptions) { - apiOptions = mergeOptions(apiOptions, newOptions); + middlewareOptions = mergeOptions(middlewareOptions, newOptions); }, getPluginOptions() { - return mergeOptions(pluginOptions, apiOptions); + return mergeOptions(pluginOptions, middlewareOptions); }, }, configResolved(resolvedConfig) { @@ -59,76 +62,71 @@ export function hydrogen(pluginOptions: HydrogenPluginOptions = {}): Plugin[] { (plugin) => plugin.name === 'oxygen:main', ); - if (oxygenPlugin) { - oxygenPlugin.api?.registerPluginOptions?.({ - shouldStartRuntime: () => !isRemixChildCompiler(resolvedConfig), - requestHook: ({request, response, meta}) => { - // Emit events for requests - emitRequestEvent( - { - __fromVite: true, - eventType: 'request', - url: request.url, - requestId: request.headers['request-id'], - purpose: request.headers['purpose'], - startTime: meta.startTimeMs, - responseInit: { - status: response.status, - statusText: response.statusText, - headers: Object.entries(response.headers), - }, - }, - resolvedConfig.root, - ); - }, - crossBoundarySetup: [ + middlewareOptions.isOxygen = !!oxygenPlugin; + + oxygenPlugin?.api?.registerPluginOptions?.({ + shouldStartRuntime: () => !isRemixChildCompiler(resolvedConfig), + requestHook: ({request, response, meta}) => { + // Emit events for requests + emitRequestEvent( { - // Setup the global function in the Oxygen worker - script: (binding) => { - globalThis.__H2O_LOG_EVENT = binding; - }, - binding: (data) => { - // Emit events for subrequests from the parent process - emitRequestEvent( - data as RequestEventPayload, - resolvedConfig.root, - ); + __fromVite: true, + eventType: 'request', + url: request.url, + requestId: request.headers['request-id'], + purpose: request.headers['purpose'], + startTime: meta.startTimeMs, + endTime: meta.endTimeMs, + responseInit: { + status: response.status, + statusText: response.statusText, + headers: Object.entries(response.headers), }, }, - /** - * To avoid initial CSS flash during development, - * most frameworks implement a way to gather critical CSS. - * Remix does this by calling a global function that their - * Vite plugin creates in the Node.js process: - * @see https://github.com/remix-run/remix/blob/b07921efd5e8eed98e2996749852777c71bc3e50/packages/remix-server-runtime/dev.ts#L37-L47 - * - * Here we are setting up a stub function in the Oxygen worker - * that will be called by Remix during development. Then, we forward - * this request to the Node.js process (Vite server) where the actual - * Remix function is called and the critical CSS is returned to the worker. - */ - { - script: (binding) => { - // Setup global dev hooks in Remix in the worker environment - // using the binding function passed from Node environment: - globalThis.__remix_devServerHooks = {getCriticalCss: binding}; - }, - binding: (...args) => { - // Call the global Remix dev hook for critical CSS in Node environment: - return globalThis.__remix_devServerHooks?.getCriticalCss?.( - ...args, - ); - }, + resolvedConfig.root, + ); + }, + crossBoundarySetup: [ + { + // Setup the global function in the Oxygen worker + script: (binding) => { + globalThis.__H2O_LOG_EVENT = binding; + }, + binding: (data) => { + // Emit events for subrequests from the parent process + emitRequestEvent( + data as RequestEventPayload, + resolvedConfig.root, + ); + }, + }, + /** + * To avoid initial CSS flash during development, + * most frameworks implement a way to gather critical CSS. + * Remix does this by calling a global function that their + * Vite plugin creates in the Node.js process: + * @see https://github.com/remix-run/remix/blob/b07921efd5e8eed98e2996749852777c71bc3e50/packages/remix-server-runtime/dev.ts#L37-L47 + * + * Here we are setting up a stub function in the Oxygen worker + * that will be called by Remix during development. Then, we forward + * this request to the Node.js process (Vite server) where the actual + * Remix function is called and the critical CSS is returned to the worker. + */ + { + script: (binding) => { + // Setup global dev hooks in Remix in the worker environment + // using the binding function passed from Node environment: + globalThis.__remix_devServerHooks = {getCriticalCss: binding}; + }, + binding: (...args) => { + // Call the global Remix dev hook for critical CSS in Node environment: + return globalThis.__remix_devServerHooks?.getCriticalCss?.( + ...args, + ); }, - ], - } satisfies OxygenApiOptions); - } else { - // If Oxygen is not present, we are probably running - // on Node.js, so we can setup the global functions directly. - globalThis.__H2O_LOG_EVENT = (data) => { - emitRequestEvent(data, resolvedConfig.root); - }; - } + }, + ], + } satisfies OxygenApiOptions); }, configureServer(viteDevServer) { if (isRemixChildCompiler(viteDevServer.config)) return; @@ -136,7 +134,7 @@ export function hydrogen(pluginOptions: HydrogenPluginOptions = {}): Plugin[] { return () => { setupHydrogenMiddleware( viteDevServer, - mergeOptions(pluginOptions, apiOptions), + mergeOptions(pluginOptions, middlewareOptions), ); }; }, diff --git a/packages/hydrogen/src/vite/request-events.ts b/packages/hydrogen/src/vite/request-events.ts index 3c343227cd..80ebe9e5fe 100644 --- a/packages/hydrogen/src/vite/request-events.ts +++ b/packages/hydrogen/src/vite/request-events.ts @@ -3,7 +3,11 @@ import {EventEmitter} from 'node:events'; import type {IncomingMessage, ServerResponse} from 'node:http'; import {mapSourcePosition} from 'source-map-support'; -const DEV_ROUTES = new Set(['/graphiql', '/subrequest-profiler']); +const DEV_ROUTES = new Set([ + '/graphiql', + '/subrequest-profiler', + '/debug-network-server', +]); type RequestEvent = { event: string;