fix(solidstart): Use nitro module for build hooks to preserve preset hooks

Previously, to move the instrumentation file to the build output we added a
`rollup:before` hook via nitro's `server.hooks`. This however overrides any
nitro preset-defined hooks.

Any preset that defines such a hook, has their hook not run. Notably, the
`aws-preset` sets up `rollup:before` to switch the entry files when using aws
streaming.

The fix is to move our hook logic into our own nitro module, which then runs
additvely on top of all the other hooks that have been defined by other presets.

Closes: #20857

Author: andreiborza

packages/solidstart/src/config/withSentry.ts

@@ -36,42 +36,44 @@ export function withSentry(
   };
 
   const server = (solidStartConfig.server || {}) as SolidStartInlineServerConfig;
-  const hooks = server.hooks || {};
   const viteConfig = solidStartConfig.vite;
   const vite =
     typeof viteConfig === 'function'
       ? (...args: Parameters<typeof viteConfig>) => addSentryPluginToVite(viteConfig(...args), sentryPluginOptions)
       : addSentryPluginToVite(viteConfig, sentryPluginOptions);
 
+  // Use a module so we don't override preset hooks.
+  const sentryNitroModule = (nitro: Nitro) => {
+    nitro.hooks.hook('rollup:before', async (nitro, rollupConfig) => {
+      if (sentrySolidStartPluginOptions?.autoInjectServerSentry === 'experimental_dynamic-import') {
+        await addDynamicImportEntryFileWrapper({
+          nitro,
+          rollupConfig: rollupConfig as unknown as RollupConfig,
+          sentryPluginOptions,
+        });
+
+        sentrySolidStartPluginOptions.debug &&
+          debug.log(
+            'Wrapping the server entry file with a dynamic `import()`, so Sentry can be preloaded before the server initializes.',
+          );
+      } else {
+        await addInstrumentationFileToBuild(nitro);
+
+        if (sentrySolidStartPluginOptions?.autoInjectServerSentry === 'top-level-import') {
+          await addSentryTopImport(nitro);
+        }
+      }
+    });
+  };
+
+  const existingModules = (server as SolidStartInlineServerConfig & { modules?: unknown[] }).modules || [];
+
   return {
     ...solidStartConfig,
     vite,
     server: {
       ...server,
-      hooks: {
-        ...hooks,
-        async 'rollup:before'(nitro: Nitro, config: RollupConfig) {
-          if (sentrySolidStartPluginOptions?.autoInjectServerSentry === 'experimental_dynamic-import') {
-            await addDynamicImportEntryFileWrapper({ nitro, rollupConfig: config, sentryPluginOptions });
-
-            sentrySolidStartPluginOptions.debug &&
-              debug.log(
-                'Wrapping the server entry file with a dynamic `import()`, so Sentry can be preloaded before the server initializes.',
-              );
-          } else {
-            await addInstrumentationFileToBuild(nitro);
-
-            if (sentrySolidStartPluginOptions?.autoInjectServerSentry === 'top-level-import') {
-              await addSentryTopImport(nitro);
-            }
-          }
-
-          // Run user provided hook
-          if (hooks['rollup:before']) {
-            hooks['rollup:before'](nitro);
-          }
-        },
-      },
+      modules: [...existingModules, sentryNitroModule],
     },
   };
 }

packages/solidstart/test/config/withSentry.test.ts

@@ -36,18 +36,41 @@ describe('withSentry()', () => {
       preset: 'vercel',
     },
   };
+  const rollupConfig = { plugins: [] };
 
-  it('adds a nitro hook to add the instrumentation file to the build if no plugin options are provided', async () => {
+  function callSentryNitroModule(config: ReturnType<typeof withSentry>): { hookFn: (...args: unknown[]) => unknown } {
+    const modules = (config?.server as { modules?: unknown[] })?.modules || [];
+    const sentryModule = modules[modules.length - 1] as (nitro: Nitro) => void;
+    let hookFn: (...args: unknown[]) => unknown = () => {};
+    const fakeNitro = {
+      ...nitroOptions,
+      hooks: {
+        hook: (_name: string, fn: (...args: unknown[]) => unknown) => {
+          hookFn = fn;
+        },
+      },
+    } as unknown as Nitro;
+    sentryModule(fakeNitro);
+    return { hookFn };
+  }
+
+  it('registers a nitro module that hooks into rollup:before to add the instrumentation file', async () => {
     const config = withSentry(solidStartConfig, {});
-    await config?.server.hooks['rollup:before'](nitroOptions);
+    const { hookFn } = callSentryNitroModule(config);
+    await hookFn(nitroOptions, rollupConfig);
     expect(addInstrumentationFileToBuildMock).toHaveBeenCalledWith(nitroOptions);
-    expect(userDefinedNitroRollupBeforeHookMock).toHaveBeenCalledWith(nitroOptions);
   });
 
-  it('adds a nitro hook to add the instrumentation file as top level import to the server entry file when configured in autoInjectServerSentry', async () => {
+  it('does not override user-defined hooks in server.hooks', () => {
+    const config = withSentry(solidStartConfig, {});
+    expect(config?.server.hooks?.['rollup:before']).toBe(userDefinedNitroRollupBeforeHookMock);
+    expect(config?.server.hooks?.close).toBe(userDefinedNitroCloseHookMock);
+  });
+
+  it('adds the instrumentation file as top level import when configured as top-level-import', async () => {
     const config = withSentry(solidStartConfig, { autoInjectServerSentry: 'top-level-import' });
-    await config?.server.hooks['rollup:before'](nitroOptions);
-    await config?.server.hooks['close'](nitroOptions);
+    const { hookFn } = callSentryNitroModule(config);
+    await hookFn(nitroOptions, rollupConfig);
     expect(addSentryTopImportMock).toHaveBeenCalledWith(
       expect.objectContaining({
         options: {
@@ -59,15 +82,13 @@ describe('withSentry()', () => {
         },
       }),
     );
-    expect(userDefinedNitroCloseHookMock).toHaveBeenCalled();
   });
 
   it('does not add the instrumentation file as top level import if autoInjectServerSentry is undefined', async () => {
     const config = withSentry(solidStartConfig, { autoInjectServerSentry: undefined });
-    await config?.server.hooks['rollup:before'](nitroOptions);
-    await config?.server.hooks['close'](nitroOptions);
+    const { hookFn } = callSentryNitroModule(config);
+    await hookFn(nitroOptions, rollupConfig);
     expect(addSentryTopImportMock).not.toHaveBeenCalled();
-    expect(userDefinedNitroCloseHookMock).toHaveBeenCalled();
   });
 
   it('adds the sentry solidstart vite plugin', () => {
@@ -134,4 +155,22 @@ describe('withSentry()', () => {
       'my-test-plugin',
     ]);
   });
+
+  it('preserves existing server modules', () => {
+    const existingModule = vi.fn();
+    const config = withSentry(
+      {
+        ...solidStartConfig,
+        server: {
+          ...solidStartConfig.server,
+          modules: [existingModule],
+        },
+      },
+      {},
+    );
+    const modules = (config?.server as { modules?: unknown[] })?.modules || [];
+    expect(modules).toHaveLength(2);
+    expect(modules[0]).toBe(existingModule);
+    expect(typeof modules[1]).toBe('function');
+  });
 });