diff --git a/AGENTS.md b/AGENTS.md index 60821b2..3f52e99 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -23,4 +23,6 @@ Review the `README.md` and `CONTRIBUTING.md` for all relevant repository informa - Test startup is slow by design — each test file starts a real Harper instance and waits for Next.js to build (up to 2 minutes). A slow start is not a failure. - The ISR cache tests in `integrationTests/next-16.pw.ts` are intentionally skipped; `CacheHandler.cts` is a work in progress. - `next-16-static-data` is run by two test files: `next-16-static-data.pw.ts` on the default VM module loader (where Harper's component `harper` allowlist omits `flushDatabases`, so the plugin's pre-build flush is a no-op and a read-only build child can't see unflushed writes) and `next-16-static-data-native.pw.ts` under `applications.moduleLoader: native`, where the flush does run. The pair is what pins that behavior down — keep both. +- `next-16-mounted` covers an application served under a Harper `urlPath`. Its assertions are `test.describe.fixme` and must be un-skipped once harper's `Request.withNodeAdapter()` presents a faithful Node request/response — see HarperFast/nextjs#61 and the reproducers in `~/dev/scripts/harper-node-adapter-repro`. Against today's harper an adapted request 500s on `headers.hasOwnProperty` and a missing `appendHeader`/`_implicitHeader`, and any response larger than the adapter's 16 KB buffer stalls with no error. Every other fixture is unmounted, so nothing rewrites its URL, it keeps the direct hand-off to Next.js, and it is unaffected. - CI is currently disabled (`if: false` in `.github/workflows/integration-tests.yml`). Run tests locally. +- The `page`-based tests need Playwright's browser binaries (`npx playwright install chromium`); without them they fail instantly with `browserType.launch: Executable doesn't exist`. The `request`-based tests do not. diff --git a/fixtures/next-16-mounted/.npmrc b/fixtures/next-16-mounted/.npmrc new file mode 100644 index 0000000..9cf9495 --- /dev/null +++ b/fixtures/next-16-mounted/.npmrc @@ -0,0 +1 @@ +package-lock=false \ No newline at end of file diff --git a/fixtures/next-16-mounted/app/about/page.js b/fixtures/next-16-mounted/app/about/page.js new file mode 100644 index 0000000..4155839 --- /dev/null +++ b/fixtures/next-16-mounted/app/about/page.js @@ -0,0 +1,3 @@ +export default function Page() { + return

Mounted About

; +} diff --git a/fixtures/next-16-mounted/app/api/echo/route.js b/fixtures/next-16-mounted/app/api/echo/route.js new file mode 100644 index 0000000..b0a6631 --- /dev/null +++ b/fixtures/next-16-mounted/app/api/echo/route.js @@ -0,0 +1,6 @@ +export const dynamic = 'force-dynamic'; + +export async function GET(request) { + const url = new URL(request.url); + return Response.json({ pathname: url.pathname, search: url.search }); +} diff --git a/fixtures/next-16-mounted/app/layout.js b/fixtures/next-16-mounted/app/layout.js new file mode 100644 index 0000000..8c73497 --- /dev/null +++ b/fixtures/next-16-mounted/app/layout.js @@ -0,0 +1,11 @@ +export const metadata = { + title: 'Harper - Mounted Next.js App', +}; + +export default function RootLayout({ children }) { + return ( + + {children} + + ); +} diff --git a/fixtures/next-16-mounted/app/page.js b/fixtures/next-16-mounted/app/page.js new file mode 100644 index 0000000..94acfbd --- /dev/null +++ b/fixtures/next-16-mounted/app/page.js @@ -0,0 +1,3 @@ +export default function Page() { + return

Mounted Home

; +} diff --git a/fixtures/next-16-mounted/config.yaml b/fixtures/next-16-mounted/config.yaml new file mode 100644 index 0000000..ca71c02 --- /dev/null +++ b/fixtures/next-16-mounted/config.yaml @@ -0,0 +1,3 @@ +'@harperfast/nextjs': + package: '@harperfast/nextjs' + urlPath: /mounted diff --git a/fixtures/next-16-mounted/next.config.ts b/fixtures/next-16-mounted/next.config.ts new file mode 100644 index 0000000..ada3f1d --- /dev/null +++ b/fixtures/next-16-mounted/next.config.ts @@ -0,0 +1,3 @@ +import { withHarper } from '@harperfast/nextjs'; + +export default withHarper({}); diff --git a/fixtures/next-16-mounted/package.json b/fixtures/next-16-mounted/package.json new file mode 100644 index 0000000..f34c71b --- /dev/null +++ b/fixtures/next-16-mounted/package.json @@ -0,0 +1,16 @@ +{ + "name": "next-16-mounted", + "private": true, + "scripts": { + "dev": "next dev", + "build": "next build", + "start": "next start", + "lint": "next lint" + }, + "dependencies": { + "@harperfast/nextjs": "file:../../", + "react": "^19", + "react-dom": "^19", + "next": "^16" + } +} diff --git a/integrationTests/next-16-mounted.pw.ts b/integrationTests/next-16-mounted.pw.ts new file mode 100644 index 0000000..c56b3e2 --- /dev/null +++ b/integrationTests/next-16-mounted.pw.ts @@ -0,0 +1,47 @@ +import { fixture } from './fixture.ts'; + +const { test, expect } = fixture('next-16-mounted'); + +// Un-skip once harper's `Request.withNodeAdapter()` can serve Next.js — today an adapted request 500s +// on `headers.hasOwnProperty` and a missing `appendHeader`/`_implicitHeader`, and any response over +// the adapter's 16 KB buffer stalls. Skipped rather than left red because with CI disabled a green +// local run is this repo's only regression signal. See HarperFast/nextjs#61. +test.describe.fixme('served under a urlPath mount', () => { + test('mount root reaches the Next.js home page', async ({ request, harper }) => { + const response = await request.get(`${harper.httpURL}/mounted`); + expect(response.status()).toBe(200); + expect(await response.text()).toContain('Mounted Home'); + }); + + test('a nested page under the mount reaches its Next.js route', async ({ request, harper }) => { + const response = await request.get(`${harper.httpURL}/mounted/about`); + expect(response.status()).toBe(200); + expect(await response.text()).toContain('Mounted About'); + }); + + test('Next.js sees the mount-relative path, not the requested one', async ({ request, harper }) => { + const response = await request.get(`${harper.httpURL}/mounted/api/echo`); + expect(response.status()).toBe(200); + expect(await response.json()).toEqual({ pathname: '/api/echo', search: '' }); + }); + + test('the query string survives mount stripping', async ({ request, harper }) => { + const response = await request.get(`${harper.httpURL}/mounted/api/echo?q=1`); + expect(response.status()).toBe(200); + expect(await response.json()).toEqual({ pathname: '/api/echo', search: '?q=1' }); + }); + + // Asserts on the server-rendered markup only: Next.js still emits its `/_next/*` asset URLs at the + // root, outside the mount, until the app is built with a matching `basePath`. + test('the mount root renders in a browser', async ({ page, harper }) => { + await page.goto(`${harper.httpURL}/mounted`); + await expect(page.locator('h1')).toHaveText('Mounted Home'); + }); +}); + +// Outside the fixme block: nothing rewrites this URL, so it never reaches the adapter. +test('paths outside the mount are not served by Next.js', async ({ request, harper }) => { + const response = await request.get(`${harper.httpURL}/about`); + expect(response.status()).toBe(404); + expect(await response.text()).not.toContain('Mounted About'); +}); diff --git a/src/plugin.ts b/src/plugin.ts index 9868b45..e04652a 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -394,12 +394,46 @@ async function serve(scope: Scope, config: NextPluginConfig, next: NextPackage) const requestHandler = app.getRequestHandler(); + let warnedMissingNodeAdapter = false; + scope.server?.http?.( (request, next) => { - return request._nodeResponse === undefined - ? next(request) - : // @ts-expect-error - Not sure when the IncomingMessage.url could be undefined ; need to dig into it. - requestHandler(request._nodeRequest, request._nodeResponse, urlParse(request._nodeRequest.url, true)); + // `== null`, not `=== undefined`: Harper's Bun and uWS requests carry a null `_nodeResponse`, + // and neither implements the Node adapter used below. + if (request._nodeResponse == null) return next(request); + // Harper's router strips an application's urlPath mount by proxying the Harper `Request`, so the + // Node request underneath it still carries the un-stripped URL. Only a request middleware + // rewrote needs the adapter, which presents the Request's method/url/headers over that Node + // request; everything else keeps the cheaper direct hand-off. + if (request.url !== request._nodeRequest.url) { + if (typeof request.withNodeAdapter === 'function') { + return request + .withNodeAdapter((nodeRequest, nodeResponse) => + // @ts-expect-error - Not sure when the IncomingMessage.url could be undefined ; need to dig into it. + requestHandler(nodeRequest, nodeResponse, urlParse(nodeRequest.url, true)) + ) + .then((response) => { + // Required by withNodeAdapter: a connection reset after the headers are sent destroys + // this stream with an error, which Node throws as an uncaught exception without a + // listener. Harper's own pipe already warns about the error, so this only has to + // exist, not report. + response.body.on('error', (error) => + scope.logger.debug?.(`Next.js response stream error for ${request.pathname}: `, error) + ); + return response; + }); + } + if (!warnedMissingNodeAdapter) { + warnedMissingNodeAdapter = true; + scope.logger.warn?.( + 'This version of Harper does not provide Request.withNodeAdapter, so Next.js receives the URL as it ' + + `arrived rather than the ${request.pathname} Harper resolved it to. An application mounted at a ` + + 'urlPath will not route correctly; upgrade Harper or serve the application at the root.' + ); + } + } + // @ts-expect-error - Not sure when the IncomingMessage.url could be undefined ; need to dig into it. + return requestHandler(request._nodeRequest, request._nodeResponse, urlParse(request._nodeRequest.url, true)); }, { runFirst: config.runFirst, port: config.port, securePort: config.securePort } );