diff --git a/AGENTS.md b/AGENTS.md index 9d46448..60821b2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,8 +18,9 @@ Review the `README.md` and `CONTRIBUTING.md` for all relevant repository informa ## Testing Tips - Use `npm link` in this directory and `npm link @harperfast/nextjs` in other project directories to test out changes locally -- Run `npm run install:fixtures` before running tests for the first time, and again after changing any fixture's `package.json`. +- Run `npm run install:fixtures` before running tests for the first time, and again after changing any fixture's `package.json` **or any plugin source under `src/`**. Fixtures install the plugin with `--install-links`, so each one holds a *copy* of `dist/`, not a symlink — `npm run build` alone does not reach them. - Run `npm run test:integration` to run all tests, or `npm run test:integration -- integrationTests/next-15.pw.ts` for a single file. - 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. - CI is currently disabled (`if: false` in `.github/workflows/integration-tests.yml`). Run tests locally. diff --git a/fixtures/next-16-static-data/.npmrc b/fixtures/next-16-static-data/.npmrc new file mode 100644 index 0000000..43c97e7 --- /dev/null +++ b/fixtures/next-16-static-data/.npmrc @@ -0,0 +1 @@ +package-lock=false diff --git a/fixtures/next-16-static-data/app/dogs/page.js b/fixtures/next-16-static-data/app/dogs/page.js new file mode 100644 index 0000000..3e873a2 --- /dev/null +++ b/fixtures/next-16-static-data/app/dogs/page.js @@ -0,0 +1,54 @@ +// A statically generated page that reads a Harper table at build time. No `force-dynamic`, no +// dynamic APIs, so Next.js prerenders it during `next build` — and Next.js runs static generation in +// a *child process*. Loading Harper there opens the same RocksDB databases the parent Harper process +// already has open, which without `HARPER_READONLY` fails with: +// +// Error: IO error: While lock file: /database/data/LOCK: Resource temporarily unavailable +// +// Real apps declare `harper` as a dependency and use the bare `import('harper')` specifier (see +// HarperFast/nextjs-example). Fixtures can't: the harness deep-copies the fixture directory into the +// Harper root and `harper` installs to ~577MB. So the test passes harper's already-resolved entry +// path in via `HARPER_FIXTURE_HARPER_ENTRY`. The mechanism under test is unchanged — a build child +// process loading Harper's storage layer against a database the parent holds locked. +const harperEntry = process.env.HARPER_FIXTURE_HARPER_ENTRY; + +async function listDogs() { + // Served from a Harper worker thread, the `tables` global is already there. In the build child it + // is not, and loading Harper is what installs it (and opens the databases). + // + // The ignore comments are required: both bundlers otherwise try to resolve this specifier at build + // time and fail with "Cannot find module as expression is too dynamic". They tell the bundler to + // leave the import alone and let Node resolve the absolute path at runtime. + if (typeof globalThis.tables === 'undefined') { + if (!harperEntry) { + throw new Error( + 'HARPER_FIXTURE_HARPER_ENTRY is not set, so this fixture cannot load Harper outside a Harper thread. ' + + 'It is passed in by integrationTests/next-16-static-data.pw.ts.' + ); + } + await import(/* webpackIgnore: true */ /* turbopackIgnore: true */ harperEntry); + } + + // No fallback if `tables.Dog` is missing, deliberately. An empty list is a *meaningful* result here + // — it is what a read-only child renders when it can't see the parent's unflushed writes — so + // swallowing a failure to load Harper as `[]` would make this test pass for the wrong reason. + const dogs = []; + for await (const dog of globalThis.tables.Dog.search()) { + dogs.push({ id: dog.id, name: dog.name }); + } + return dogs; +} + +export default async function Page() { + const dogs = await listDogs(); + + return ( + + ); +} diff --git a/fixtures/next-16-static-data/app/layout.js b/fixtures/next-16-static-data/app/layout.js new file mode 100644 index 0000000..d64c151 --- /dev/null +++ b/fixtures/next-16-static-data/app/layout.js @@ -0,0 +1,11 @@ +export const metadata = { + title: 'Harper - Next.js v16 Static Data App', +}; + +export default function RootLayout({ children }) { + return ( + + {children} + + ); +} diff --git a/fixtures/next-16-static-data/app/page.js b/fixtures/next-16-static-data/app/page.js new file mode 100644 index 0000000..8080041 --- /dev/null +++ b/fixtures/next-16-static-data/app/page.js @@ -0,0 +1,3 @@ +export default function Page() { + return

Next.js v16 Static Data

; +} diff --git a/fixtures/next-16-static-data/config.yaml b/fixtures/next-16-static-data/config.yaml new file mode 100644 index 0000000..da8288c --- /dev/null +++ b/fixtures/next-16-static-data/config.yaml @@ -0,0 +1,12 @@ +rest: true + +graphqlSchema: + files: schema.graphql + +# Seeds the Dog table. Declared before @harperfast/nextjs so the row is committed by the time the +# plugin runs `next build` — the statically generated /dogs page reads it during prerender. +jsResource: + files: seed.js + +'@harperfast/nextjs': + package: '@harperfast/nextjs' diff --git a/fixtures/next-16-static-data/next.config.mjs b/fixtures/next-16-static-data/next.config.mjs new file mode 100644 index 0000000..ada3f1d --- /dev/null +++ b/fixtures/next-16-static-data/next.config.mjs @@ -0,0 +1,3 @@ +import { withHarper } from '@harperfast/nextjs'; + +export default withHarper({}); diff --git a/fixtures/next-16-static-data/package.json b/fixtures/next-16-static-data/package.json new file mode 100644 index 0000000..6b606f5 --- /dev/null +++ b/fixtures/next-16-static-data/package.json @@ -0,0 +1,16 @@ +{ + "name": "next-16-static-data", + "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/fixtures/next-16-static-data/schema.graphql b/fixtures/next-16-static-data/schema.graphql new file mode 100644 index 0000000..46bc292 --- /dev/null +++ b/fixtures/next-16-static-data/schema.graphql @@ -0,0 +1,4 @@ +type Dog @table @export { + id: ID @primaryKey + name: String +} diff --git a/fixtures/next-16-static-data/seed.js b/fixtures/next-16-static-data/seed.js new file mode 100644 index 0000000..001dac4 --- /dev/null +++ b/fixtures/next-16-static-data/seed.js @@ -0,0 +1,5 @@ +import { tables } from 'harper'; + +// Top-level await: Harper awaits component module evaluation, so the row is committed before the +// '@harperfast/nextjs' plugin (declared after this file in config.yaml) starts `next build`. +await tables.Dog.put({ id: 'rex', name: 'Rex' }); diff --git a/integrationTests/fixture.ts b/integrationTests/fixture.ts index e1aeef6..3ebc0c4 100644 --- a/integrationTests/fixture.ts +++ b/integrationTests/fixture.ts @@ -15,10 +15,27 @@ function getHarperBinPath(): string { return join(dirname(require.resolve('harper')), 'bin', 'harper.js'); } +/** + * Absolute path to Harper's module entry, for fixtures whose app code needs to import Harper from a + * `next build` child process. Fixtures can't depend on `harper` themselves — the harness deep-copies + * the fixture directory and the package is ~577MB — so the resolved path is passed through the + * environment instead. See fixtures/next-16-static-data/app/dogs/page.js. + */ +export function harperModuleEntry(): string { + return require.resolve('harper'); +} + // Next.js build can take a while — give it 2 minutes. const STARTUP_TIMEOUT_MS = 120_000; -export function makeHarperFixture(fixtureName: string) { +export interface FixtureOptions { + /** Extra environment variables for the Harper process (inherited by its workers and their children). */ + env?: Record; + /** Overrides merged over the default `applications` config, e.g. `{ moduleLoader: 'native' }`. */ + applications?: Record; +} + +export function makeHarperFixture(fixtureName: string, { env, applications }: FixtureOptions = {}) { return async ({}: {}, use: (harper: HarperContext) => Promise) => { const fixturePath = join(import.meta.dirname, '..', 'fixtures', fixtureName); const ctx = createHarperContext(fixtureName); @@ -28,6 +45,7 @@ export function makeHarperFixture(fixtureName: string) { started = await setupHarperWithFixture(ctx, fixturePath, { harperBinPath: getHarperBinPath(), startupTimeoutMs: STARTUP_TIMEOUT_MS, + ...(env && { env }), config: { logging: { stdStreams: true, @@ -36,7 +54,8 @@ export function makeHarperFixture(fixtureName: string) { lockdown: 'none', moduleLoader: 'none', dependencyLoader: 'native', - allowedDirectory: 'any' + allowedDirectory: 'any', + ...applications, }, }, }); @@ -55,9 +74,9 @@ export function makeHarperFixture(fixtureName: string) { }; } -export function fixture(fixtureName: string) { +export function fixture(fixtureName: string, options?: FixtureOptions) { const test = base.extend<{}, { harper: HarperContext }>({ - harper: [makeHarperFixture(fixtureName), { scope: 'worker', timeout: 120_000 }], + harper: [makeHarperFixture(fixtureName, options), { scope: 'worker', timeout: 120_000 }], }); return { test, expect }; } diff --git a/integrationTests/next-16-static-data-native.pw.ts b/integrationTests/next-16-static-data-native.pw.ts new file mode 100644 index 0000000..a673fcc --- /dev/null +++ b/integrationTests/next-16-static-data-native.pw.ts @@ -0,0 +1,32 @@ +import { fixture, harperModuleEntry } from './fixture.ts'; + +// The same fixture as next-16-static-data.pw.ts, under `applications.moduleLoader: native`. +// +// Harper's default VM loader hands components a `harper` module built from a fixed allowlist +// (`getHarperExports` in harper's security/jsLoader.ts) that omits `flushDatabases`, so the plugin's +// pre-build flush is a no-op and a read-only build child can't see writes the parent hasn't flushed. +// `native` mode skips the VM loader entirely and resolves the real package, so the flush runs — which +// makes this the only place the suite actually covers `flushDatabases()` doing its job. +// +// The trade-off is real and documented: native mode gives up per-app tagged logging and `config`. +const { test, expect } = fixture('next-16-static-data', { + env: { HARPER_FIXTURE_HARPER_ENTRY: harperModuleEntry() }, + applications: { moduleLoader: 'native' }, +}); + +// The counterpart to 'without a reachable flush...' in next-16-static-data.pw.ts: same page, same +// seeded row, but the flush ran before `next build`, so the statically generated page sees the row. +test('a reachable flush lets the build child see writes committed before the build', async ({ page, harper }) => { + await page.goto(`${harper.httpURL}/dogs`); + await expect(page.getByTestId('dog-rex')).toHaveText('Rex'); +}); + +test('Harper still accepts writes after the build', async ({ request, harper }) => { + const headers = { + authorization: `Basic ${Buffer.from(`${harper.admin.username}:${harper.admin.password}`).toString('base64')}`, + 'content-type': 'application/json', + }; + + const response = await request.put(`${harper.httpURL}/Dog/fido`, { data: { name: 'Fido' }, headers }); + expect(response.ok()).toBe(true); +}); diff --git a/integrationTests/next-16-static-data.pw.ts b/integrationTests/next-16-static-data.pw.ts new file mode 100644 index 0000000..1db3b1c --- /dev/null +++ b/integrationTests/next-16-static-data.pw.ts @@ -0,0 +1,51 @@ +import { fixture, harperModuleEntry } from './fixture.ts'; + +// Regression coverage for Harper read-only mode during `next build`. +// +// Next.js runs static generation in child processes. When a prerendered page reads Harper data, that +// child loads Harper's storage layer and opens the same RocksDB databases the parent Harper process +// already holds — which fails on the LOCK file unless the build sets `HARPER_READONLY`. Every other +// fixture either never touches Harper data or marks its data pages `force-dynamic`, so nothing else +// in this suite covers the build-time path. +const { test, expect } = fixture('next-16-static-data', { + env: { HARPER_FIXTURE_HARPER_ENTRY: harperModuleEntry() }, +}); + +test('home page renders', async ({ page, harper }) => { + await page.goto(harper.httpURL); + await expect(page.locator('h1')).toHaveText('Next.js v16 Static Data'); +}); + +// The core assertion. Without `HARPER_READONLY` the prerender of /dogs throws on the RocksDB LOCK, +// `next build` exits non-zero, and the plugin never reaches `serve()` — so no route exists at all and +// this fails on a connection error rather than a missing element. Reaching a rendered list means the +// build child opened the locked databases read-only. +test('statically generated page builds while Harper holds the databases open', async ({ page, harper }) => { + await page.goto(`${harper.httpURL}/dogs`); + await expect(page.getByTestId('dogs')).toBeAttached(); +}); + +// Documents the flush gap under the default (VM) loader, rather than skipping it. Harper's component +// `harper` module is a fixed allowlist (security/jsLoader.ts) that omits `flushDatabases`, so the +// pre-build flush is a no-op here: the read-only child reads on-disk state and prerenders an empty +// list even though `GET /Dog/rex` returns Rex. next-16-static-data-native.pw.ts runs the same fixture +// with `moduleLoader: native`, where the flush does run and the row *is* visible. +test('without a reachable flush, the build child does not see unflushed writes', async ({ page, harper }) => { + await page.goto(`${harper.httpURL}/dogs`); + await expect(page.getByTestId('dogs')).toBeEmpty(); +}); + +// The build must not leave the serving process read-only, or writes after the build fail. +test('Harper still accepts writes after the build', async ({ request, harper }) => { + const headers = { + authorization: `Basic ${Buffer.from(`${harper.admin.username}:${harper.admin.password}`).toString('base64')}`, + 'content-type': 'application/json', + }; + + const response = await request.put(`${harper.httpURL}/Dog/fido`, { data: { name: 'Fido' }, headers }); + expect(response.ok()).toBe(true); + + const created = await request.get(`${harper.httpURL}/Dog/fido`, { headers }); + expect(created.status()).toBe(200); + expect((await created.json()).name).toBe('Fido'); +}); diff --git a/scripts/install-fixtures.js b/scripts/install-fixtures.js index f7b6b92..8262384 100644 --- a/scripts/install-fixtures.js +++ b/scripts/install-fixtures.js @@ -1,6 +1,6 @@ #!/usr/bin/env node -import { readdirSync } from 'node:fs'; +import { readdirSync, rmSync } from 'node:fs'; import { join, dirname } from 'node:path'; import { fileURLToPath } from 'node:url'; import { spawnSync } from 'node:child_process'; @@ -13,8 +13,18 @@ const fixtures = readdirSync(fixturesDir, { withFileTypes: true }) for (const fixture of fixtures) { console.log(`Installing ${fixture} dependencies...`); + const fixtureDir = join(fixturesDir, fixture); + + // `--install-links` copies `file:../..` instead of symlinking it, so the fixture holds a snapshot + // of the plugin rather than a live view. npm considers that snapshot to satisfy the tree and + // restores it from its cache on reinstall, so a rebuilt `dist/` never reaches the fixture and + // tests silently run stale plugin code. Dropping the copy *and* npm's hidden lockfile forces a + // fresh one — without removing `.package-lock.json` npm just re-extracts the cached version. + rmSync(join(fixtureDir, 'node_modules', '@harperfast'), { recursive: true, force: true }); + rmSync(join(fixtureDir, 'node_modules', '.package-lock.json'), { force: true }); + spawnSync('npm', ['install', '--install-links'], { - cwd: join(fixturesDir, fixture), + cwd: fixtureDir, stdio: 'inherit', }); console.log('\n'); diff --git a/src/plugin.ts b/src/plugin.ts index e9353e9..9868b45 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -1,4 +1,14 @@ -import { flushDatabases, type Scope, type Config, type FilesOption } from 'harper'; +// Namespace import, not `import { flushDatabases }`. Harper loads components through a sandboxed VM +// loader where `harper` is a SyntheticModule built from a hardcoded allowlist (getHarperExports in +// harper's security/jsLoader.ts). `flushDatabases` is a documented package export but is not on that +// allowlist, so a named import fails at *link* time with "does not provide an export named +// 'flushDatabases'" — fatal, taking the whole plugin down. A namespace import always links; the +// property is simply undefined where it isn't exposed, which runNextBuild handles. +// +// It *is* defined under `applications.moduleLoader: native`, which bypasses the VM loader and resolves +// the real package. So this has to work both ways, not just on the default loader. +import * as harper from 'harper'; +import type { Scope, Config, FilesOption } from 'harper'; import { createRequire } from 'node:module'; import { parse as urlParse } from 'node:url'; @@ -282,9 +292,29 @@ async function runNextBuild(scope: Scope, config: NextPluginConfig, next: NextPa // RocksDB databases, so force child processes to start Harper in read-only mode. Flush first // so child processes can see all committed data. Unset after the build so the server process // itself is not permanently locked into read-only mode. + // + // The flush is best-effort in both directions: it isn't reachable on every Harper build (see the + // import comment), and it can fail on a transient storage error. Read-only children still open the + // databases and replay the on-disk WAL, so a missing flush only risks them not seeing the most + // recent writes — worth a warning, not worth failing the build. Nothing here may throw: this runs + // before the try/finally below, so an escaping error would leave HARPER_READONLY set on a thread + // that goes on serving requests. const prevHarperReadonly = process.env.HARPER_READONLY; process.env.HARPER_READONLY = 'true'; - await flushDatabases(); + if (harper.flushDatabases) { + try { + await harper.flushDatabases(); + } catch (error) { + scope.logger.warn?.('Failed to flush databases before build; static pages may not see the most recent writes: ', error); + } + } else { + scope.logger.warn?.( + 'harper.flushDatabases is unavailable, so the pre-build flush was skipped and statically generated pages ' + + 'may not see recently written data. Harper only exposes it to components under the native module ' + + 'loader; set `applications.moduleLoader: native` in harperdb-config.yaml to enable the flush, at the ' + + 'cost of per-application tagged logging and config.' + ); + } // --expose-internals is set in Harper's worker execArgv but is not allowed in NODE_OPTIONS. // Next.js reads process.execArgv to forward flags to its own child workers via NODE_OPTIONS,