test: add Next.js integration test for server-util (#942)#2555
test: add Next.js integration test for server-util (#942)#2555
Conversation
Adds a minimal Next.js test app and integration test that verifies ServerBlockNoteEditor works in Next.js API routes with serverExternalPackages. Tests shared schema (createReactBlockSpec) between API routes and client editor pages. Also adds a production build mode CI step (NEXTJS_TEST_MODE=build). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a Next.js test app, packaging/setup script for local Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Integration Test
participant Setup as setup.sh
participant NextJS as Next.js Server
participant API as /api/server-util
participant Editor as /editor Page
Test->>Setup: run setup.sh (pack tarballs, update package.json, npm install)
Setup-->>Test: setup complete
Test->>NextJS: start server (dev or build+start)
NextJS-->>Test: ready signal (stdout)
Test->>API: GET /api/server-util
API->>API: instantiate ServerBlockNoteEditor, run rendering & conversions, run Yjs roundtrip
API-->>Test: JSON { allPassed, results } (200/500)
Test->>Editor: GET /editor
Editor-->>Test: HTML page (contains "BlockNote Editor Test" and editor wrapper)
Test->>NextJS: terminate process
NextJS-->>Test: exited
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/nextjs-test-app/.gitignore`:
- Line 4: .tests/nextjs-test-app currently ignores package-lock.json while
tests/nextjs-test-app/package.json uses floating caret ranges for next, react,
react-dom, `@mantine/core` and `@mantine/hooks`, making the fixture
non-reproducible; fix by removing the "package-lock.json" entry from .gitignore
and committing the generated tests/nextjs-test-app/package-lock.json so CI
installs exact versions, or alternatively pin those dependencies in
tests/nextjs-test-app/package.json to exact versions (replace caret ranges) and
then commit the lockfile accordingly.
In `@tests/nextjs-test-app/app/api/server-util/route.tsx`:
- Line 10: The import of schema in route.tsx violates the repo import/extensions
rule; update the specifier that currently reads import { schema } from
"../../shared-schema" to include the correct file extension (matching the actual
module file, e.g. "../../shared-schema.ts") so the linter stops flagging it;
ensure the referenced symbol "schema" and the import in route.tsx are updated
consistently.
In `@tests/nextjs-test-app/next.config.ts`:
- Around line 4-6: The test fixture globally disables TypeScript checking via
the typescript.ignoreBuildErrors setting; remove that (delete or set to false)
in next.config.ts and instead fix the underlying type errors in the fixture
files by adding proper ambient module declarations (e.g., declare module
"@blocknote"; declare module "next/dynamic"; add missing React type imports) or
apply targeted local suppressions with `@ts-expect-error` where a single-line
suppression is appropriate; keep typescript.ignoreBuildErrors disabled so next
build fails on regressions and the CI test validates real type correctness.
In `@tests/nextjs-test-app/setup.sh`:
- Around line 14-18: The current CURRENT_HASH only hashes dist/ files so changes
to package manifests slip through; update the hashing logic that computes
CURRENT_HASH (and still checks HASH_FILE) to also include each package's
package.json (e.g., "$PACKAGES_DIR/core/package.json",
"$PACKAGES_DIR/react/package.json", "$PACKAGES_DIR/server-util/package.json",
"$PACKAGES_DIR/mantine/package.json") or, alternatively, derive the key via npm
pack --json --dry-run for each package and hash that output — ensure the symbols
HASH_FILE, CURRENT_HASH, and PACKAGES_DIR are used to locate and replace the
existing find/cat pipeline so the new manifest-aware hash prevents reuse of
stale tarballs.
- Around line 10-12: The REPO_ROOT resolution uses git rev-parse
--git-common-dir which returns the shared .git for linked worktrees, causing
PACKAGES_DIR to point to the main checkout; change the command in the REPO_ROOT
assignment to use git rev-parse --show-toplevel (with git -C "$SCRIPT_DIR") so
REPO_ROOT resolves to the current worktree root and PACKAGES_DIR points at the
correct packages/ tree.
In `@tests/src/unit/nextjs/serverUtil.test.ts`:
- Around line 114-117: The empty catch around JSON.parse(nextDataMatch[1]) hides
parse failures and trips the no-empty lint rule; update the try/catch in the
block that sets errorMessage and the similar block at lines 145-147 to make
parse failures explicit (either call a small helper like safeJsonParse(str) that
returns undefined on parse errors, or keep the try but add a comment or minimal
body inside catch such as `/* ignore invalid nextData JSON */` or `return
undefined;`) so that JSON.parse failures are handled without an empty catch and
the code references the same logic for nextDataMatch and errorMessage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9be3c151-f327-4867-92c5-17a083db4fe6
📒 Files selected for processing (12)
.github/workflows/build.ymltests/nextjs-test-app/.gitignoretests/nextjs-test-app/app/api/server-util/route.tsxtests/nextjs-test-app/app/editor/Editor.tsxtests/nextjs-test-app/app/editor/page.tsxtests/nextjs-test-app/app/layout.tsxtests/nextjs-test-app/app/shared-schema.tsxtests/nextjs-test-app/next.config.tstests/nextjs-test-app/package.jsontests/nextjs-test-app/setup.shtests/nextjs-test-app/tsconfig.jsontests/src/unit/nextjs/serverUtil.test.ts
| node_modules | ||
| .next | ||
| .tarballs | ||
| package-lock.json |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Ignored lockfile:"
git check-ignore -v tests/nextjs-test-app/package-lock.json || true
echo
echo "Floating dependency ranges in tests/nextjs-test-app/package.json:"
python - <<'PY'
import json
with open("tests/nextjs-test-app/package.json", "r", encoding="utf-8") as f:
deps = json.load(f)["dependencies"]
for name, version in deps.items():
if version.startswith("^") or version.startswith("~"):
print(f"{name}: {version}")
PYRepository: TypeCellOS/BlockNote
Length of output: 336
Don't ignore the fixture lockfile.
Line 4 ignores package-lock.json while tests/nextjs-test-app/package.json uses floating caret ranges for next, react, react-dom, @mantine/core, and @mantine/hooks. This makes the fixture non-reproducible—CI will install whatever versions match the ranges at build time, causing unrelated failures or masking regressions this fixture is meant to catch. Commit the lockfile, or pin those dependencies exactly.
📌 Suggested change
node_modules
.next
.tarballs
-package-lock.json
next-env.d.ts📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| package-lock.json | |
| node_modules | |
| .next | |
| .tarballs | |
| next-env.d.ts |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/nextjs-test-app/.gitignore` at line 4, .tests/nextjs-test-app currently
ignores package-lock.json while tests/nextjs-test-app/package.json uses floating
caret ranges for next, react, react-dom, `@mantine/core` and `@mantine/hooks`,
making the fixture non-reproducible; fix by removing the "package-lock.json"
entry from .gitignore and committing the generated
tests/nextjs-test-app/package-lock.json so CI installs exact versions, or
alternatively pin those dependencies in tests/nextjs-test-app/package.json to
exact versions (replace caret ranges) and then commit the lockfile accordingly.
There was a problem hiding this comment.
Won't fix — setup.sh regenerates package.json with tarball paths on every run, so the lockfile changes each time. Floating ranges for third-party deps are intentional so we catch breakages.
There was a problem hiding this comment.
@YousefED, understood — since setup.sh rewrites package.json with tarball file: paths on each run the lockfile is inherently ephemeral, and keeping floating ranges is a deliberate choice to surface incompatibilities with newer versions of the third-party deps. Makes sense, I'll drop this suggestion.
(─‿─) 🐇
✏️ Learnings added
Learnt from: YousefED
Repo: TypeCellOS/BlockNote PR: 2555
File: tests/nextjs-test-app/.gitignore:4-4
Timestamp: 2026-03-11T20:28:50.385Z
Learning: In `tests/nextjs-test-app/`, the `package-lock.json` is intentionally excluded from version control because `tests/nextjs-test-app/setup.sh` regenerates `package.json` with local tarball `file:` paths on every run, making the lockfile change each time. Floating caret version ranges for third-party deps (`next`, `react`, `react-dom`, `mantine/core`, `mantine/hooks`) are intentional so CI catches breakages against newer versions. Do not flag the missing/ignored lockfile as an issue.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| // } from "@blocknote/core"; | ||
| // import { createReactBlockSpec } from "@blocknote/react"; | ||
| // import { createContext, useContext } from "react"; | ||
| import { schema } from "../../shared-schema"; |
There was a problem hiding this comment.
Align this import with the repo's extension rule.
This specifier currently trips import/extensions, so pnpm run lint will fail unless the test app is excluded or the import matches the configured extension policy.
🧰 Tools
🪛 ESLint
[error] 10-10: Missing file extension "tsx" for "../../shared-schema"
(import/extensions)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/nextjs-test-app/app/api/server-util/route.tsx` at line 10, The import
of schema in route.tsx violates the repo import/extensions rule; update the
specifier that currently reads import { schema } from "../../shared-schema" to
include the correct file extension (matching the actual module file, e.g.
"../../shared-schema.ts") so the linter stops flagging it; ensure the referenced
symbol "schema" and the import in route.tsx are updated consistently.
There was a problem hiding this comment.
Not applicable — Next.js/TypeScript doesn't require .tsx extensions on relative imports. The test app isn't subject to the workspace lint config.
There was a problem hiding this comment.
@YousefED, understood — the tests/nextjs-test-app/ directory is a standalone Next.js app not subject to the workspace ESLint config, and TypeScript/Next.js projects don't require explicit .tsx extensions on relative imports. I'll note this for future reviews.
✏️ Learnings added
Learnt from: YousefED
Repo: TypeCellOS/BlockNote PR: 2555
File: tests/nextjs-test-app/app/api/server-util/route.tsx:10-10
Timestamp: 2026-03-11T20:28:48.964Z
Learning: The `tests/nextjs-test-app/` directory in the BlockNote repository is a standalone Next.js test application that is NOT subject to the workspace-level ESLint configuration. Do not flag `import/extensions` lint errors for relative imports inside `tests/nextjs-test-app/`, as Next.js and TypeScript projects do not require explicit file extensions (e.g., `.tsx`) on relative imports.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| typescript: { | ||
| ignoreBuildErrors: true, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Next.js typescript.ignoreBuildErrorsallownext build to succeed even when the app contains TypeScript errors, and is it intended only as a last resort?
💡 Result:
Yes. Setting typescript.ignoreBuildErrors: true tells Next.js to let next build complete even if there are TypeScript errors (i.e., it skips the built-in type-checking step for production builds). [1]
It’s explicitly described as dangerous (“dangerously allow production builds to successfully complete even if your project has type errors”) and the docs warn you should only do this if you’re running type checks separately (e.g., tsc --noEmit in CI), which makes it a last-resort/escape hatch rather than a recommended default. [2]
Sources:
[1] https://nextjs.org/docs/pages/api-reference/config/next-config-js/typescript
[2] https://nextjs.org/docs/app/api-reference/config/typescript
🏁 Script executed:
cat -n tests/nextjs-test-app/next.config.tsRepository: TypeCellOS/BlockNote
Length of output: 426
🏁 Script executed:
# Check for TypeScript errors in the fixture
cd tests/nextjs-test-app && find . -name "*.ts" -o -name "*.tsx" | head -20Repository: TypeCellOS/BlockNote
Length of output: 199
🏁 Script executed:
# Check the git log for context on why this was added
git log --oneline -20 -- tests/nextjs-test-app/next.config.tsRepository: TypeCellOS/BlockNote
Length of output: 141
🏁 Script executed:
# Get details about the commit that added this config
git show 7e4d771a --statRepository: TypeCellOS/BlockNote
Length of output: 50377
🏁 Script executed:
# Check if there are actual TypeScript errors in the app
cd tests/nextjs-test-app && npx tsc --noEmit 2>&1 | head -50Repository: TypeCellOS/BlockNote
Length of output: 1756
🏁 Script executed:
# Also check the PR/commit message for more context
git show 7e4d771a | head -100Repository: TypeCellOS/BlockNote
Length of output: 3165
Let the build test fail on TypeScript regressions.
Disabling typescript.ignoreBuildErrors weakens the new build-mode integration test. The fixture has real TypeScript errors (missing module declarations for @blocknote, next/dynamic, React types, etc.). Enabling this setting means next build will succeed even if these fixtures stop type-checking against Next.js or BlockNote, eliminating a key validation that the new CI step is meant to provide. Fix the errors directly, or suppress them locally with @ts-expect-error, rather than disabling type-checking globally.
📌 Suggested change
const nextConfig: NextConfig = {
- typescript: {
- ignoreBuildErrors: true,
- },
serverExternalPackages: [
"@blocknote/core",
"@blocknote/react",
"@blocknote/server-util",
],
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/nextjs-test-app/next.config.ts` around lines 4 - 6, The test fixture
globally disables TypeScript checking via the typescript.ignoreBuildErrors
setting; remove that (delete or set to false) in next.config.ts and instead fix
the underlying type errors in the fixture files by adding proper ambient module
declarations (e.g., declare module "@blocknote"; declare module "next/dynamic";
add missing React type imports) or apply targeted local suppressions with
`@ts-expect-error` where a single-line suppression is appropriate; keep
typescript.ignoreBuildErrors disabled so next build fails on regressions and the
CI test validates real type correctness.
There was a problem hiding this comment.
Won't fix — the TS errors come from tarball type declarations not matching the test app's tsconfig. The test validates runtime behavior, not type correctness.
There was a problem hiding this comment.
@YousefED, understood — that's a reasonable trade-off given the tarballs are local packs whose type declarations may not perfectly align with the test app's tsconfig. Since the fixture exists only to validate runtime behavior in Next.js App Router, suppressing type-check failures globally makes sense here. Acknowledged.
✏️ Learnings added
Learnt from: YousefED
Repo: TypeCellOS/BlockNote PR: 2555
File: tests/nextjs-test-app/next.config.ts:4-6
Timestamp: 2026-03-11T20:28:45.166Z
Learning: In `tests/nextjs-test-app/next.config.ts`, `typescript.ignoreBuildErrors: true` is intentional. The Next.js test fixture installs `blocknote/*` packages as locally packed tarballs (via `setup.sh`), which causes TypeScript type declaration mismatches with the test app's tsconfig. The fixture is designed solely to validate runtime behavior in Next.js App Router server contexts, not type correctness, so this setting is expected and should not be flagged in future reviews.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Also removes outdated React 19 StrictMode note from Next.js page. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
YousefED
force-pushed
the
fix/942-nextjs-server-util
branch
from
1f6d663 to
f779c14
Compare
- Fix empty catch blocks (no-empty) and jest/valid-expect lint errors - Use --show-toplevel in setup.sh so worktrees resolve independently - Include package.json manifests in tarball cache hash Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@blocknote/ariakit
@blocknote/code-block
@blocknote/core
@blocknote/mantine
@blocknote/react
@blocknote/server-util
@blocknote/shadcn
@blocknote/xl-ai
@blocknote/xl-docx-exporter
@blocknote/xl-email-exporter
@blocknote/xl-multi-column
@blocknote/xl-odt-exporter
@blocknote/xl-pdf-exporter
commit: |
1 participant
Summary
Adds an integration test that verifies
@blocknote/server-utilworks correctly in Next.js App Router server contexts (API routes), addressing #942.Rationale
@blocknote/server-utilmust work in Next.js API routes for users who do server-side block conversion (HTML, Markdown, Yjs). This was previously untested and broken without theserverExternalPackagesconfig. This PR documents the required config and adds a regression test.Changes
tests/nextjs-test-app/— Minimal Next.js app with:next.config.tswithserverExternalPackagesfor@blocknote/core,@blocknote/react,@blocknote/server-util/api/server-util) mirroringReactServer.test.tsx— testsblocksToFullHTMLwith custom React blocks,blocksToHTMLLossy, and Yjs roundtripshared-schema.tsx) usingcreateReactBlockSpec, imported by both the API route and a client editor page — verifies the same schema works in both contextsdynamic(() => import("./Editor"), { ssr: false })with the shared schemasetup.shthat packs@blocknote/*as tarballs and installs them (with hash-based caching for fast re-runs)tests/src/unit/nextjs/serverUtil.test.ts— Integration test that runssetup.sh, spawns a Next.js server, and hits both the API route and editor page. SupportsNEXTJS_TEST_MODE=dev(default, fast) andNEXTJS_TEST_MODE=build(production build).github/workflows/build.yml— Added production build mode CI step after unit testsImpact
No changes to any
@blocknote/*package source code. The fix for users is purely anext.config.tsconfiguration change (serverExternalPackages).The context block test from
ReactServer.test.tsxis commented out in the API route because React's server bundle forbidscreateContextat runtime — this is a React restriction, not a BlockNote one. The context block test remains covered byReactServer.test.tsxin the regular vitest suite.Testing
npx vitest run tests/src/unit/nextjs/serverUtil.test.ts— dev mode (~5s cached, ~30s cold)NEXTJS_TEST_MODE=build npx vitest run tests/src/unit/nextjs/serverUtil.test.ts— production build mode (~40s)Checklist
Additional Notes
Users need to add this to their
next.config.ts:Closes #942
Summary by CodeRabbit
Tests
New Features
Chores
Documentation