diff --git a/.changeset/builder-package.md b/.changeset/builder-package.md new file mode 100644 index 0000000000..626e8c6e2a --- /dev/null +++ b/.changeset/builder-package.md @@ -0,0 +1,27 @@ +--- +"nextly": patch +"create-nextly-app": patch +"@nextlyhq/admin": patch +"@nextlyhq/admin-css": patch +"@nextlyhq/blocks-engine": patch +"@nextlyhq/blocks-react": patch +"@nextlyhq/ui": patch +"@nextlyhq/adapter-drizzle": patch +"@nextlyhq/adapter-postgres": patch +"@nextlyhq/adapter-mysql": patch +"@nextlyhq/adapter-sqlite": patch +"@nextlyhq/storage-s3": patch +"@nextlyhq/storage-uploadthing": patch +"@nextlyhq/storage-vercel-blob": patch +"@nextlyhq/plugin-form-builder": patch +"@nextlyhq/plugin-page-builder": patch +"@nextlyhq/plugin-seo": patch +"@nextlyhq/plugin-sdk": patch +"@nextlyhq/eslint-config": patch +"@nextlyhq/prettier-config": patch +"@nextlyhq/telemetry": patch +"@nextlyhq/tsconfig": patch +"@nextlyhq/builder": patch +--- + +Add the @nextlyhq/builder package, which will hold the visual page-builder editor. It ships no features yet, so there is nothing to install it for: it exists now so the editor arrives under a name that is already reserved and already versioned in lockstep with the rest. It requires React 19, matching the renderer it draws with (@nextlyhq/blocks-react). diff --git a/.changeset/config.json b/.changeset/config.json index b71d0c536a..33f708e465 100644 --- a/.changeset/config.json +++ b/.changeset/config.json @@ -30,7 +30,8 @@ "@nextlyhq/eslint-config", "@nextlyhq/prettier-config", "@nextlyhq/telemetry", - "@nextlyhq/tsconfig" + "@nextlyhq/tsconfig", + "@nextlyhq/builder" ] ], "linked": [], diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9b4dfd425c..68a6fe9d96 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -147,6 +147,7 @@ jobs: pnpm turbo test --filter=@nextlyhq/blocks-engine --filter=@nextlyhq/blocks-react + --filter=@nextlyhq/builder --filter=@nextlyhq/plugin-page-builder --filter=@nextlyhq/plugin-form-builder --filter=@nextlyhq/plugin-seo diff --git a/.github/workflows/pr-title.yml b/.github/workflows/pr-title.yml index dfb6e7bea9..5099b949f9 100644 --- a/.github/workflows/pr-title.yml +++ b/.github/workflows/pr-title.yml @@ -48,6 +48,7 @@ jobs: storage-uploadthing blocks-engine blocks-react + builder plugin-form-builder plugin-page-builder plugin-seo diff --git a/AGENTS.md b/AGENTS.md index 881b9c4652..e5f0fbd4b6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -19,6 +19,13 @@ for all published packages. Status: alpha, all packages version in lockstep. documents; its root entry imports no `next/*`, no admin and no CMS runtime, so it is usable standalone (enforced by `src/layering.test.ts`). Next-coupled helpers live at the `/next` subpath. +- `packages/builder` - the visual page-builder editor (shell, canvas, op store). + It reaches admin only through `plugin-sdk/admin`, and imports nothing outside a + short allowlist of exact specifiers - both enforced by `src/layering.test.ts`. + That it draws with `blocks-react` rather than a renderer of its own is a + review-time convention, NOT a checked one: reimplementing rendering on React + and `blocks-engine` imports exactly the same packages, so no import guard can + tell the two apart. - `packages/plugin-sdk` - the ONLY stable import surface for plugin authors. - `packages/plugin-{form-builder,page-builder}` - first-party plugins. - `packages/storage-{s3,vercel-blob,uploadthing}` - media storage adapters. @@ -74,7 +81,7 @@ Before editing a package, read its README.md and check for a nested AGENTS.md. `adapter-postgres`, `adapter-mysql`, `adapter-sqlite`, `adapter-drizzle`, `storage-s3`, `storage-vercel-blob`, `storage-uploadthing`, `plugin-form-builder`, `plugin-page-builder`, `plugin-seo`, `plugin-sdk`, - `blocks-engine`, `blocks-react`, + `blocks-engine`, `blocks-react`, `builder`, `create-nextly-app`, `eslint-config`, `prettier-config`, `tsconfig`, `telemetry`, `client`) plus `playground`, `root`, `ci`, `docs`, `deps`, `release`. Scope is optional; the subject must not start with an uppercase diff --git a/eslint.config.mjs b/eslint.config.mjs index ffc58d15ca..88bec5f51e 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -12,6 +12,7 @@ const REACT_FILES = [ "packages/admin/**/*.{ts,tsx,js,jsx}", "packages/ui/**/*.{ts,tsx,js,jsx}", "packages/blocks-react/**/*.{ts,tsx,js,jsx}", + "packages/builder/**/*.{ts,tsx,js,jsx}", "apps/playground/**/*.{ts,tsx,js,jsx}", ]; diff --git a/packages/builder/README.md b/packages/builder/README.md new file mode 100644 index 0000000000..46f9b8ea26 --- /dev/null +++ b/packages/builder/README.md @@ -0,0 +1,93 @@ +# @nextlyhq/builder + +The visual page-builder editor: the shell, the canvas, and the op store that +everything in it either produces or reads. + +**It ships no features yet.** The package exists ahead of them so its name is +claimed on npm — trusted publishing cannot perform a package's first publish, and +the bootstrap script will not claim a name that is not already a workspace +package. There is nothing to install it for until the editor lands. + +## What this package is not + +**It is not a renderer.** The canvas draws documents through +`@nextlyhq/blocks-react` — the same renderer that serves published pages — and +re-implements nothing downstream of the document model. Read-path preparation, +condition gating and slot pruning are consumed from the engine's own entry +points, never reproduced here. + +That rule is not stylistic. `plugin-page-builder` carries a second renderer of +its own, and the two disagree about condition gating in opposite directions: one +fails closed, the other does not evaluate conditions at all. Sharing a predicate +would not have prevented that, because sharing a predicate does not share the +decision to call it. Only sharing the entry point does. + +## The layering contract + +`src/layering.test.ts` enforces two boundaries as a build failure: + +**The strongest enforcement is not the scan.** `@nextlyhq/admin` is in no +dependency field of this package, and pnpm's `node_modules` is not hoisted, so it +does not resolve here at all — verified, `MODULE_NOT_FOUND`. Every spelling of a +direct admin import therefore fails to build, including spellings TypeScript has +not shipped yet. A test asserts the manifest stays that way, which is complete by +construction in a way a syntax scan can never be. + +The scan is the second layer: it fails at test time naming the rule, rather than +at build time with a resolution error, and it is the **only** enforcement for the +subpath policy below — `blocks-react/next`, the `plugin-sdk` root and +`plugin-sdk/testing` all resolve perfectly, because their packages are legitimate +dependencies. The graph has nothing to say about which _entry_ of a dependency is +allowed. + +1. **Never `@nextlyhq/admin` directly.** Admin is reached only through + `@nextlyhq/plugin-sdk/admin`, a curated facade where every export is named + individually and carries a stability tag. A direct import bypasses the facade + and takes a dependency on internals nobody promised to keep. The pull toward + it is concrete: the editor wants admin's Lexical node set for inline rich + text. +2. **Never the CMS runtime.** The allowlist is **exact specifiers**, not + packages, because subpaths are where the coupling lives — + `@nextlyhq/blocks-react/next` imports `nextly/runtime`, and the + `@nextlyhq/plugin-sdk` root re-exports runtime values from `nextly`. Only + `@nextlyhq/plugin-sdk/admin` is admitted, and only the root entry of + `blocks-react`. + +The guard reads every shape that reaches a module — static imports and +re-exports, `import()`, bare `require()`, `import x = require()`, +`typeof import()`, and triple-slash type references — because each of those has, +at some point, walked straight past a narrower version of it. + +**What the guard does NOT prove:** that the canvas renders _through_ +`blocks-react` rather than reimplementing rendering on React and +`blocks-engine`. Both spellings import exactly the same packages, so no import +scan can separate them. That rule is held by review. The allowlist makes the +shortcut inconvenient; it cannot make it impossible. + +Adding an allowlist entry is a deliberate act with a reason recorded beside it. + +## Public surface + +`BUILDER_PACKAGE_NAME` — this package's npm name, for diagnostics that report +which packages a host loaded. The name and not the version: a version literal in +source would be stale one release after it was written, because every release +bumps this package in lockstep with its siblings. + +## Development + +Run these from this directory (`packages/builder`), not the repository root — +turbo swallows the summary line at the root. + +```bash +pnpm run test # vitest, including the layering guard +pnpm run check-types # tsc --noEmit; unlike some packages here, this DOES + # cover the test files (tsconfig has no test exclude) +pnpm run lint # eslint --max-warnings 0; a single warning fails +pnpm run build # tsup +``` + +## Peer dependencies + +React 19, matching the renderer it draws with. `@nextlyhq/blocks-react` requires +`react: ^19.0.0`, and it is a dependency here rather than a peer, so a React 18 +host would hit an unsatisfiable peer one level down. diff --git a/packages/builder/eslint.config.js b/packages/builder/eslint.config.js new file mode 100644 index 0000000000..ed79d4c38c --- /dev/null +++ b/packages/builder/eslint.config.js @@ -0,0 +1,8 @@ +// The builder is a React package — it renders THROUGH `@nextlyhq/blocks-react` +// rather than being a renderer — so it layers the shared React rules over the +// repository base. The root config scopes those rules by path; this per-package +// config applies them unconditionally, matching how the other React packages +// lint when run directly. +import { config } from "@nextlyhq/eslint-config/react-internal"; + +export default [...config]; diff --git a/packages/builder/package.json b/packages/builder/package.json new file mode 100644 index 0000000000..0803b476d8 --- /dev/null +++ b/packages/builder/package.json @@ -0,0 +1,79 @@ +{ + "name": "@nextlyhq/builder", + "version": "0.0.2-alpha.55", + "description": "The visual page-builder editor for Nextly: shell, canvas and op store.", + "license": "MIT", + "repository": { + "type": "git", + "url": "git+https://github.com/nextlyhq/nextly.git", + "directory": "packages/builder" + }, + "homepage": "https://nextlyhq.com/docs", + "bugs": { + "url": "https://github.com/nextlyhq/nextly/issues" + }, + "type": "module", + "main": "dist/index.mjs", + "module": "dist/index.mjs", + "types": "dist/index.d.ts", + "exports": { + ".": { + "types": "./dist/index.d.ts", + "import": "./dist/index.mjs", + "default": "./dist/index.mjs" + } + }, + "files": [ + "dist" + ], + "scripts": { + "build": "tsup", + "dev": "tsup --watch", + "check-types": "tsc --noEmit", + "lint": "eslint . --max-warnings 0", + "lint:fix": "eslint . --fix", + "test": "vitest run", + "test:watch": "vitest", + "clean": "rimraf dist" + }, + "engines": { + "node": ">=20.0.0" + }, + "dependencies": { + "@nextlyhq/blocks-engine": "workspace:*", + "@nextlyhq/blocks-react": "workspace:*" + }, + "peerDependencies": { + "@nextlyhq/plugin-sdk": "workspace:*", + "@nextlyhq/ui": "workspace:^", + "react": "^19.0.0", + "react-dom": "^19.0.0" + }, + "devDependencies": { + "@nextlyhq/eslint-config": "workspace:*", + "@nextlyhq/plugin-sdk": "workspace:*", + "@nextlyhq/tsconfig": "workspace:*", + "@nextlyhq/ui": "workspace:*", + "@types/node": "^20.19.17", + "@types/react": "19.2.0", + "@types/react-dom": "19.2.0", + "eslint": "^9.39.1", + "react": "19.2.0", + "react-dom": "19.2.0", + "rimraf": "^6.1.3", + "tsup": "^8.5.0", + "typescript": "^5.9.3", + "vitest": "^4.1.0" + }, + "publishConfig": { + "access": "public", + "registry": "https://registry.npmjs.org/", + "provenance": true + }, + "keywords": [ + "nextly", + "page-builder", + "visual-editor", + "canvas" + ] +} diff --git a/packages/builder/src/index.ts b/packages/builder/src/index.ts new file mode 100644 index 0000000000..eed78064d0 --- /dev/null +++ b/packages/builder/src/index.ts @@ -0,0 +1,43 @@ +/** + * `@nextlyhq/builder` — the visual page-builder editor. + * + * The editor half of the page builder: the shell, the canvas, and the op store + * that everything in it either produces or reads. It is deliberately NOT a + * renderer. + * + * **The invariant this package is built around**: the canvas renders documents + * through `@nextlyhq/blocks-react` — the same renderer that serves published + * pages — and re-implements nothing downstream of the document model. Read-path + * preparation, condition gating and slot pruning are consumed from the engine's + * own entry points, never reproduced here. + * + * It is held by review rather than by a test. Reimplementing rendering on React + * and the engine imports exactly the same packages as delegating to the + * renderer, so the layering guard cannot tell the two apart; it narrows what may + * be imported, which makes the shortcut inconvenient rather than impossible. + * + * That rule is not stylistic. `plugin-page-builder` carries a second renderer of + * its own, and the two disagree about condition gating in OPPOSITE directions — + * one failing closed, the other not evaluating conditions at all. Sharing a + * predicate would not have prevented that, because sharing a predicate does not + * share the decision to call it; only sharing the entry point does. + * + * This entry exports no features yet. The package exists ahead of them so its + * name is claimed on npm: trusted publishing cannot perform a package's first + * publish, and the bootstrap script will not claim a name that is not already a + * workspace package. + * + * @module @nextlyhq/builder + */ + +/** + * This package's npm name, for diagnostics that report which packages a host + * has loaded. + * + * The name and not the version. A version literal in source would be stale one + * release after it was written, because every release bumps this package in + * lockstep with its siblings; reporting a version means injecting the manifest's + * value at build time, which belongs with the surface that displays it rather + * than with a constant nothing reads yet. + */ +export const BUILDER_PACKAGE_NAME = "@nextlyhq/builder" as const; diff --git a/packages/builder/src/layering.test.ts b/packages/builder/src/layering.test.ts new file mode 100644 index 0000000000..e400b6a92f --- /dev/null +++ b/packages/builder/src/layering.test.ts @@ -0,0 +1,506 @@ +import { readFileSync, readdirSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +import ts from "typescript"; +import { describe, expect, it } from "vitest"; + +/** + * The package's layering contract, enforced rather than documented. + * + * Two promises, each broken by an import that is individually reasonable: + * + * 1. **The builder never imports `@nextlyhq/admin` directly.** Plugins and + * libraries reach admin only through `@nextlyhq/plugin-sdk/admin` — a curated + * facade where every export is named individually and carries a stability + * tag. A direct import bypasses the facade and takes a dependency on + * internals nobody promised to keep. + * + * The pull toward it is real rather than hypothetical: the editor wants + * admin's Lexical node set for inline rich text, and reaching for it directly + * is the shape that looks harmless at the call site. + * + * 2. **The builder does not pull in the CMS runtime.** It draws with + * `@nextlyhq/blocks-react`'s root entry, which is standalone; the `/next` + * subpath is the Next-coupled one and imports `nextly/runtime`. Admitting it + * here would put the whole server runtime behind an editor component. + * + * The guard is an ALLOWLIST of exact specifiers. A blocklist only stops what + * someone thought to name, and a rule written per PACKAGE rather than per + * specifier silently admits every subpath a package happens to publish — which + * is how `blocks-react/next` and `plugin-sdk/testing` would arrive. Adding an + * entry below is a deliberate act with a reason recorded beside it. + * + * **What this file does NOT prove.** That the canvas renders THROUGH + * `blocks-react` rather than reimplementing rendering on top of React and + * `@nextlyhq/blocks-engine` is a property of what the code does, not of what it + * imports, and both spellings import exactly the same packages. The allowlist + * makes the shortcut inconvenient; it cannot make it impossible. Treat that rule + * as a design constraint reviewed by people, not as one enforced here. + */ + +// `import.meta.dirname` only exists from Node 20.11 and the package floor is +// Node >=20.0, so derive the directory from the module URL instead. +const SRC_DIR = dirname(fileURLToPath(import.meta.url)); + +/** + * What the package may import at runtime, and why. + * + * - `react`, `react-dom`, `react/jsx-runtime`: the editor is a React app. Peer + * dependencies, so the host's copy is the only one in the tree. + * - `@nextlyhq/blocks-engine`: the document model, validation and style + * compiler. Runtime-free. + * - `@nextlyhq/blocks-react`: the renderer the canvas draws with — the same one + * that serves published pages. + * - `@nextlyhq/ui`: the design system, and the only source of admin-theme + * tokens. Peer, for the same one-copy reason as React. + * - `@nextlyhq/plugin-sdk`: the stable import surface, including the `/admin` + * subpath that is the ONLY sanctioned route to admin components. + */ +const ALLOWED_RUNTIME_IMPORTS = [ + "react", + "react-dom", + "react/jsx-runtime", + "@nextlyhq/blocks-engine", + // The ROOT entry only. `@nextlyhq/blocks-react/next` imports `nextly/runtime`, + // and `/blocks` is the built-in catalogue, which nothing here needs yet. + "@nextlyhq/blocks-react", + "@nextlyhq/ui", + // The `/admin` subpath ONLY, and not the SDK root. The root re-exports runtime + // values from `nextly` (`export { definePlugin } from "nextly"`), which that + // package's build leaves external, so importing it loads the CMS runtime this + // guard exists to keep out. `/admin` re-exports from `@nextlyhq/admin` alone + // and is the one sanctioned route to admin components. Nothing here imports + // the root today; when the editor needs SDK types, add a type-only route + // deliberately rather than widening this entry back. + "@nextlyhq/plugin-sdk/admin", +]; + +/** Node built-ins and test-only tooling, which never reach a consumer's bundle. */ +const ALLOWED_IN_TESTS = [ + "node:fs", + "node:path", + "node:url", + "typescript", + "vitest", +]; + +/** + * Stands in for a module call whose target is not a literal, such as + * `import(base + name)` or `require(name)`. + * + * Such a target cannot be resolved by reading the file, so the honest report is + * "unknown", and unknown has to be a violation: the alternative is a guard that + * approves whatever it could not read. It is deliberately not a legal package + * specifier, so it can never be satisfied by an allowlist entry. + */ +const UNRESOLVABLE_SPECIFIER = ""; + +/** + * Extensions the bundler will follow, and therefore the ones this guard must read. + * + * Not just `.ts`/`.tsx`. A TypeScript entry can side-effect-import `./bridge.js`, and tsup + * bundles it; a scan restricted to TypeScript would walk past the one file free to import + * anything, with the typecheck none the wiser because `allowJs` is off. + */ +const BUNDLED_MODULE = /\.(?:tsx?|jsx?|mjs|cjs)$/; + +function sourceFiles(dir: string): string[] { + const out: string[] = []; + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const full = join(dir, entry.name); + if (entry.isDirectory()) out.push(...sourceFiles(full)); + else if (BUNDLED_MODULE.test(entry.name)) out.push(full); + } + return out; +} + +/** + * Every module specifier a source text imports, read from the AST rather than by + * regex. + * + * Several shapes reach a module, not one, and a visitor that reads only + * declarations walks straight past most of them — approving a file that pulls + * the forbidden package in anyway: + * + * - `import ... from` and `export ... from`, which carry a module specifier. + * - `import("pkg")` and `require("pkg")`, which are call expressions. A bare + * `require` identifier only: `loader.require("x")` is a method on some object, + * not a module resolve. + * - `import x = require("pkg")`, the documented CommonJS-interop spelling, which + * is neither of the above. + * - `typeof import("pkg")` in type position, which the parser gives as an + * `ImportTypeNode` rather than a call. It erases at build, so a purely runtime + * guard would skip it — this one does not, for the reason below. + * + * Template literals with no substitutions are as statically known as quoted + * strings, so they count as literals here. + * + * Type-only imports are collected too, which is stricter than a purely runtime + * guard would be. The admin prohibition is not only about what reaches a bundle: + * importing admin's types is the same dependency on internals nobody promised to + * keep, and it is one rename away from becoming a value import. + * + * Separated from the file reading so the shapes above can be asserted against + * source text directly: the contract tests below scan real files, and a file + * that happens to contain none of a shape cannot demonstrate the shape is seen. + */ +function importsOfSource(text: string, fileName = "module.ts"): string[] { + const source = ts.createSourceFile( + fileName, + text, + ts.ScriptTarget.ESNext, + true + ); + const found: string[] = []; + const seen = new Set(); + const visit = (node: ts.Node): void => { + if (seen.has(node)) return; + seen.add(node); + if ( + (ts.isImportDeclaration(node) || ts.isExportDeclaration(node)) && + node.moduleSpecifier && + ts.isStringLiteralLike(node.moduleSpecifier) + ) { + found.push(node.moduleSpecifier.text); + } else if (ts.isJSDocImportTag(node)) { + // `/** @import { X } from "pkg" */`. A tag with its own module specifier, not the + // `ImportTypeNode` a `@typedef` produces, so entering the JSDoc tree is not enough. + const target = node.moduleSpecifier; + found.push( + target && ts.isStringLiteralLike(target) + ? target.text + : UNRESOLVABLE_SPECIFIER + ); + } else if (ts.isImportTypeNode(node)) { + // `type A = typeof import("pkg")`. A type query, so it never reaches a bundle — but it is + // still a dependency on that package's internals, which is what the admin rule forbids. + const target = node.argument; + found.push( + ts.isLiteralTypeNode(target) && ts.isStringLiteralLike(target.literal) + ? target.literal.text + : UNRESOLVABLE_SPECIFIER + ); + } else if ( + ts.isImportEqualsDeclaration(node) && + ts.isExternalModuleReference(node.moduleReference) + ) { + const target = node.moduleReference.expression; + found.push( + ts.isStringLiteralLike(target) ? target.text : UNRESOLVABLE_SPECIFIER + ); + } else if (ts.isCallExpression(node)) { + const callee = node.expression; + const resolvesAModule = + callee.kind === ts.SyntaxKind.ImportKeyword || + (ts.isIdentifier(callee) && callee.text === "require"); + if (resolvesAModule) { + const target = node.arguments[0]; + found.push( + target && ts.isStringLiteralLike(target) + ? target.text + : UNRESOLVABLE_SPECIFIER + ); + } + } + ts.forEachChild(node, visit); + // JSDoc hangs off a node rather than sitting under it, so `forEachChild` never enters it. + // In a JavaScript file that is where the types live: `@typedef {import("pkg").T}` puts an + // ImportTypeNode inside the comment, invisible to every branch above. + for (const doc of ts.getJSDocCommentsAndTags(node)) visit(doc); + }; + visit(source); + + // `/// ` is not part of the node tree, so `forEachChild` never + // reaches it. The parser puts it here instead, and it is a dependency on that package's + // types exactly as an `import type` is. + for (const directive of source.typeReferenceDirectives) { + found.push(directive.fileName); + } + + return found; +} + +/** Every module specifier a file imports. */ +function importsOf(file: string): string[] { + return importsOfSource(readFileSync(file, "utf8"), file); +} + +/** + * Whether one specifier may be imported. + * + * Exact match, deliberately. Judging by package would let any subpath in, and the subpaths are + * exactly where the coupling lives: `blocks-react/next` carries `nextly/runtime` and + * `plugin-sdk/testing` is not a production surface. A permitted subpath is written out in full. + */ +function isAllowed(specifier: string, inTest: boolean): boolean { + const allowed = inTest + ? [...ALLOWED_RUNTIME_IMPORTS, ...ALLOWED_IN_TESTS] + : ALLOWED_RUNTIME_IMPORTS; + return allowed.includes(specifier); +} + +/** Bare package specifiers only — relative paths are this package's own code. */ +function isBare(specifier: string): boolean { + return !specifier.startsWith(".") && !specifier.startsWith("/"); +} + +describe("reading a module's imports", () => { + it("sees static imports and re-exports", () => { + expect(importsOfSource(`import { a } from "react";`)).toEqual(["react"]); + expect(importsOfSource(`export { b } from "@nextlyhq/ui";`)).toEqual([ + "@nextlyhq/ui", + ]); + expect(importsOfSource(`export * from "@nextlyhq/blocks-react";`)).toEqual([ + "@nextlyhq/blocks-react", + ]); + }); + + it("sees type-only imports, which a rename could turn into a value import", () => { + expect( + importsOfSource(`import type { P } from "@nextlyhq/admin";`) + ).toEqual(["@nextlyhq/admin"]); + }); + + it("sees dynamic imports, which carry a dependency no declaration records", () => { + expect( + importsOfSource(`const m = await import("@nextlyhq/admin/lexical");`) + ).toEqual(["@nextlyhq/admin/lexical"]); + expect( + importsOfSource("const m = await import(`@nextlyhq/admin`);") + ).toEqual(["@nextlyhq/admin"]); + }); + + it("sees a bare require, which reaches a module exactly as an import does", () => { + expect(importsOfSource(`const a = require("@nextlyhq/admin");`)).toEqual([ + "@nextlyhq/admin", + ]); + }); + + it("sees a triple-slash type reference, which is not in the node tree at all", () => { + // The parser stores these on `typeReferenceDirectives`, so a visitor built on + // `forEachChild` cannot reach one however carefully it is written. + expect( + importsOfSource(`/// \nexport {};`) + ).toEqual(["@nextlyhq/admin"]); + }); + + it("sees a JSDoc @import tag, which carries its own module specifier", () => { + // Distinct from the `@typedef` form: `@import` parses to a JSDocImportTag, so walking into + // JSDoc reaches it but no ImportTypeNode branch records it. Asserted against a subpath the + // guard genuinely must catch, since admin is unresolvable here anyway. + expect( + importsOfSource( + `/** @import { N } from "@nextlyhq/blocks-react/next" */\nexport const x = 1;`, + "probe.js" + ) + ).toContain("@nextlyhq/blocks-react/next"); + }); + + it("sees an import type inside a JSDoc typedef, which JavaScript files use", () => { + // JSDoc is attached to a node, not nested under it, so `forEachChild` walks past the whole + // comment. This only became reachable once the enumerator started scanning `.js` files. + expect( + importsOfSource( + `/** @typedef {import("@nextlyhq/admin").Node} Node */\nexport const x = 1;`, + "probe.js" + ) + ).toContain("@nextlyhq/admin"); + }); + + it("sees a typeof-import type query, which no call expression covers", () => { + // The parser gives this as an ImportTypeNode, so a visitor watching for calls and declarations + // walks past it. It erases at build, which is exactly why it is an easy way to take a + // dependency on admin internals without appearing to import anything. + expect( + importsOfSource(`type A = typeof import("@nextlyhq/admin");`) + ).toEqual(["@nextlyhq/admin"]); + expect( + importsOfSource(`let x: import("@nextlyhq/admin/lexical").Node;`) + ).toEqual(["@nextlyhq/admin/lexical"]); + }); + + it("sees the CommonJS-interop import-equals spelling", () => { + expect( + importsOfSource(`import admin = require("@nextlyhq/admin");`) + ).toEqual(["@nextlyhq/admin"]); + }); + + it("does not count a method that merely happens to be named require", () => { + // `loader.require("x")` resolves nothing; treating it as an import would make the guard + // fail CLOSED on innocent code, which gets guards deleted rather than obeyed. + expect(importsOfSource(`loader.require("@nextlyhq/admin");`)).toEqual([]); + }); + + it("reports a require it cannot resolve rather than dropping it", () => { + expect(importsOfSource(`const a = require(name);`)).toEqual([ + UNRESOLVABLE_SPECIFIER, + ]); + }); + + it("reports a dynamic import it cannot resolve rather than dropping it", () => { + // The failure this replaces is silent: an unreadable target that produced no + // entry left the allowlist with nothing to reject. + expect(importsOfSource(`const m = await import(name);`)).toEqual([ + UNRESOLVABLE_SPECIFIER, + ]); + expect( + importsOfSource("const m = await import(`@nextlyhq/${pkg}`);") + ).toEqual([UNRESOLVABLE_SPECIFIER]); + }); + + it("rejects an unresolved dynamic import through the same allowlist as a named one", () => { + // The sentinel is only useful if it survives the two filters between the + // reader and the verdict: it must look bare, and must match no entry. + expect(isBare(UNRESOLVABLE_SPECIFIER)).toBe(true); + expect(ALLOWED_RUNTIME_IMPORTS).not.toContain(UNRESOLVABLE_SPECIFIER); + expect(ALLOWED_IN_TESTS).not.toContain(UNRESOLVABLE_SPECIFIER); + }); + + it("ignores relative imports of the package's own code", () => { + expect( + importsOfSource(`import { x } from "./canvas";`).filter(isBare) + ).toEqual([]); + }); +}); + +describe("what the allowlist admits", () => { + // Asserted on specifiers directly rather than through the file scan: the rule that matters is + // about subpaths no file in this package imports yet, and a contract test over real source can + // only demonstrate rules its own source happens to exercise. + + it("admits the packages the editor is built from", () => { + for (const specifier of [ + "react", + "react-dom", + "react/jsx-runtime", + "@nextlyhq/blocks-engine", + "@nextlyhq/blocks-react", + "@nextlyhq/ui", + ]) { + expect(isAllowed(specifier, false)).toBe(true); + } + }); + + it("admits the one sanctioned route to admin", () => { + expect(isAllowed("@nextlyhq/plugin-sdk/admin", false)).toBe(true); + }); + + it("refuses the Next-coupled renderer entry, which carries the CMS runtime", () => { + // `@nextlyhq/blocks-react/next` imports `nextly/runtime`. Judging by package rather than by + // specifier would have admitted it on the strength of the root entry being allowed. + expect(isAllowed("@nextlyhq/blocks-react/next", false)).toBe(false); + expect(isAllowed("@nextlyhq/blocks-react/blocks", false)).toBe(false); + }); + + it("refuses the SDK root, which re-exports runtime values from nextly", () => { + // `@nextlyhq/plugin-sdk` re-exports `definePlugin` from `nextly`, and the SDK build leaves + // `nextly` external, so the root drags the CMS runtime into the editor graph. The `/admin` + // subpath re-exports from `@nextlyhq/admin` alone and stays allowed. + expect(isAllowed("@nextlyhq/plugin-sdk", false)).toBe(false); + expect(isAllowed("@nextlyhq/plugin-sdk/admin", false)).toBe(true); + }); + + it("refuses other subpaths of an allowed package", () => { + expect(isAllowed("@nextlyhq/plugin-sdk/testing", false)).toBe(false); + expect(isAllowed("@nextlyhq/plugin-sdk/client", false)).toBe(false); + }); + + it("refuses admin under every spelling", () => { + expect(isAllowed("@nextlyhq/admin", false)).toBe(false); + expect(isAllowed("@nextlyhq/admin/lexical", false)).toBe(false); + }); + + it("keeps test-only tooling out of shipped code", () => { + // The one asymmetry in the list, so it is worth pinning in both directions. + expect(isAllowed("vitest", true)).toBe(true); + expect(isAllowed("vitest", false)).toBe(false); + expect(isAllowed("node:fs", true)).toBe(true); + expect(isAllowed("node:fs", false)).toBe(false); + }); +}); + +describe("the dependency graph, which is the stronger half of the admin rule", () => { + // A package cannot import what it does not depend on. `@nextlyhq/admin` is in no dependency + // field here, and pnpm's node_modules is not hoisted, so every spelling of a direct admin + // import — dynamic, `require`, `import x = require`, `typeof import`, a JSDoc typedef, a + // triple-slash reference, in a `.js` file, cached or not — fails to RESOLVE. That is a + // completeness the source scan cannot claim: the scan covers the syntaxes someone thought of, + // this covers all of them at once, including syntaxes TypeScript has not shipped yet. + // + // The scan is still worth having. It fails at test time with a message that names the rule + // rather than at build time with a resolution error, and it is the ONLY enforcement for the + // subpath policy: `blocks-react/next`, the `plugin-sdk` root and `plugin-sdk/testing` all + // resolve perfectly, because their packages are legitimate dependencies — the graph has + // nothing to say about which ENTRY of a dependency is allowed. + + const manifest: { + dependencies?: Record; + devDependencies?: Record; + peerDependencies?: Record; + optionalDependencies?: Record; + } = JSON.parse(readFileSync(join(SRC_DIR, "..", "package.json"), "utf8")); + + const declared = [ + ...Object.keys(manifest.dependencies ?? {}), + ...Object.keys(manifest.devDependencies ?? {}), + ...Object.keys(manifest.peerDependencies ?? {}), + ...Object.keys(manifest.optionalDependencies ?? {}), + ]; + + it("reads a manifest that actually declares things", () => { + // Positive control. An unparsed or empty manifest would satisfy the assertion below while + // proving nothing, which is the failure mode this file has already paid for once. + expect(declared).toContain("@nextlyhq/blocks-react"); + }); + + it("does not depend on @nextlyhq/admin in any form", () => { + expect( + declared.filter( + name => + name === "@nextlyhq/admin" || name.startsWith("@nextlyhq/admin/") + ) + ).toEqual([]); + }); +}); + +describe("the builder's layering contract", () => { + const files = sourceFiles(SRC_DIR); + + it("reads its own source, so the assertions below are not vacuous", () => { + // Positive control. An empty file list would satisfy every `every()` below, + // and a guard that passes because it found nothing is the failure mode this + // program has paid for repeatedly. + expect(files.length).toBeGreaterThan(0); + expect(files.some(f => f.endsWith("index.ts"))).toBe(true); + }); + + it("never imports @nextlyhq/admin directly", () => { + // The one route to admin is `@nextlyhq/plugin-sdk/admin`. Asserted as a + // prefix so `@nextlyhq/admin/anything` is caught too — a subpath import is + // the shape this would most plausibly take. + const offenders = files.filter(file => + importsOf(file).some( + specifier => + specifier === "@nextlyhq/admin" || + specifier.startsWith("@nextlyhq/admin/") + ) + ); + + expect(offenders).toEqual([]); + }); + + it("imports only what the contract allows", () => { + const violations: string[] = []; + for (const file of files) { + const inTest = /\.test\.tsx?$/.test(file); + for (const specifier of importsOf(file).filter(isBare)) { + if (!isAllowed(specifier, inTest)) { + violations.push(`${file}: ${specifier}`); + } + } + } + + expect(violations).toEqual([]); + }); +}); diff --git a/packages/builder/tsconfig.json b/packages/builder/tsconfig.json new file mode 100644 index 0000000000..5466525bad --- /dev/null +++ b/packages/builder/tsconfig.json @@ -0,0 +1,11 @@ +{ + "extends": "@nextlyhq/tsconfig/base-bundler.json", + "compilerOptions": { + "rootDir": "src", + "outDir": "dist", + "jsx": "react-jsx", + "lib": ["ES2022", "DOM"] + }, + "include": ["src/**/*"], + "exclude": ["dist", "node_modules"] +} diff --git a/packages/builder/tsup.config.ts b/packages/builder/tsup.config.ts new file mode 100644 index 0000000000..f3cf3c51ca --- /dev/null +++ b/packages/builder/tsup.config.ts @@ -0,0 +1,28 @@ +import { defineConfig } from "tsup"; + +/** + * One entry for now; the externals are the contract. + * + * React and the design system belong to the host application. Bundling either + * would put a second copy in the consumer's tree — for React that breaks hooks + * outright, and for `@nextlyhq/ui` it would mean two sets of the CSS custom + * properties the admin theme is built on. + */ +export default defineConfig({ + entry: ["src/index.ts"], + format: ["esm"], + dts: true, + clean: true, + sourcemap: true, + treeshake: true, + external: [ + "react", + "react-dom", + "react/jsx-runtime", + "@nextlyhq/ui", + "@nextlyhq/plugin-sdk", + ], + outExtension() { + return { js: ".mjs" }; + }, +}); diff --git a/packages/builder/vitest.config.ts b/packages/builder/vitest.config.ts new file mode 100644 index 0000000000..a762c9b760 --- /dev/null +++ b/packages/builder/vitest.config.ts @@ -0,0 +1,21 @@ +import { defineConfig } from "vitest/config"; + +/** + * The suite is static analysis over source files, not rendering. + * + * `node` rather than a DOM environment because nothing here mounts a + * component: the layering checks read files and walk the import graph, and a + * DOM environment would add startup cost for capability none of them use. When + * renderer tests arrive they will need `jsdom`, and switching then is a + * deliberate change rather than an inherited default. + * + * `.tsx` is included alongside `.ts` because blocks are React components and + * their tests will live beside them. + */ + +export default defineConfig({ + test: { + environment: "node", + include: ["src/**/*.test.ts", "src/**/*.test.tsx"], + }, +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6dff8de06c..97acd92a14 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -823,6 +823,58 @@ importers: specifier: ^4.1.0 version: 4.1.10(@types/node@20.19.17)(@vitest/coverage-v8@4.1.10)(@vitest/ui@4.1.10)(happy-dom@20.9.0)(jsdom@27.1.0)(vite@7.3.6(@types/node@20.19.17)(jiti@2.7.0)(lightningcss@1.32.0)(terser@5.44.1)(tsx@4.21.0)(yaml@2.8.3)) + packages/builder: + dependencies: + '@nextlyhq/blocks-engine': + specifier: workspace:* + version: link:../blocks-engine + '@nextlyhq/blocks-react': + specifier: workspace:* + version: link:../blocks-react + devDependencies: + '@nextlyhq/eslint-config': + specifier: workspace:* + version: link:../eslint-config + '@nextlyhq/plugin-sdk': + specifier: workspace:* + version: link:../plugin-sdk + '@nextlyhq/tsconfig': + specifier: workspace:* + version: link:../tsconfig + '@nextlyhq/ui': + specifier: workspace:* + version: link:../ui + '@types/node': + specifier: ^20.19.17 + version: 20.19.17 + '@types/react': + specifier: 19.2.0 + version: 19.2.0 + '@types/react-dom': + specifier: 19.2.0 + version: 19.2.0(@types/react@19.2.0) + eslint: + specifier: ^9.39.1 + version: 9.39.1(jiti@2.7.0) + react: + specifier: 19.2.0 + version: 19.2.0 + react-dom: + specifier: 19.2.0 + version: 19.2.0(react@19.2.0) + rimraf: + specifier: ^6.1.3 + version: 6.1.3 + tsup: + specifier: ^8.5.0 + version: 8.5.0(jiti@2.7.0)(postcss@8.5.23)(tsx@4.21.0)(typescript@5.9.3)(yaml@2.8.3) + typescript: + specifier: ^5.9.3 + version: 5.9.3 + vitest: + specifier: ^4.1.0 + version: 4.1.10(@types/node@20.19.17)(@vitest/coverage-v8@4.1.10)(@vitest/ui@4.1.10)(happy-dom@20.9.0)(jsdom@27.1.0)(vite@7.3.6(@types/node@20.19.17)(jiti@2.7.0)(lightningcss@1.32.0)(terser@5.44.1)(tsx@4.21.0)(yaml@2.8.3)) + packages/create-nextly-app: dependencies: '@clack/prompts': @@ -13339,7 +13391,7 @@ snapshots: '@humanfs/node': 0.16.6 '@humanwhocodes/module-importer': 1.0.1 '@humanwhocodes/retry': 0.4.3 - '@types/estree': 1.0.8 + '@types/estree': 1.0.9 ajv: 6.15.0 chalk: 4.1.2 cross-spawn: 7.0.6 diff --git a/scripts/release/first-publish-acknowledged.json b/scripts/release/first-publish-acknowledged.json index 71fa481346..95e3460cb9 100644 --- a/scripts/release/first-publish-acknowledged.json +++ b/scripts/release/first-publish-acknowledged.json @@ -18,5 +18,5 @@ "Once a package has published a real version the entry stops doing anything and", "preflight will say so; remove it then." ], - "packages": [] + "packages": ["@nextlyhq/builder"] } diff --git a/turbo.jsonc b/turbo.jsonc index 87293fde50..0b0587107f 100644 --- a/turbo.jsonc +++ b/turbo.jsonc @@ -286,6 +286,11 @@ "src/**/*.{ts,tsx}", "src/**/*.test.{ts,tsx}", "src/**/*.spec.{ts,tsx}", + // JavaScript is a test input too. A bundler follows a side-effect import + // into a `.js` sibling and ships it, and a guard that reads those files + // is cached against a hash that never saw them — so the check would be + // skipped in exactly the commit that changed what it checks. + "src/**/*.{js,jsx,mjs,cjs}", // Some tests read CSS at runtime (e.g. the token-contrast check parses // theme.css), so CSS is a real test input; without it a CSS-only change // could reuse a cached pass and skip the assertion.