From b7aa125637d47708146af50649cb0eeb2c564d3a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 13:27:11 +0000 Subject: [PATCH 1/2] fix(layout): declare the load-time registration in `sideEffects` (#3899) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `packages/layout/package.json` declared `"sideEffects": false` while `src/index.ts` registers six component keys as a module load side effect. A bundler honouring the manifest is right to delete a side-effect-only `import '@object-ui/layout';` — measured with the repo's own bundler, the resulting bundle is 0 bytes with zero registrations, on a green build with no warning. Narrow the declaration to the modules that actually register, derived from the manifest's own entry fields plus the workspace source alias two in-repo consumers bundle through: ["./dist/index.js", "./dist/index.umd.cjs", "./src/index.ts"] The auto-registration itself is deliberately untouched; replacing it with an explicit API is the opposite direction and is left to the maintainer. Adds a build-level pin that runs a real bundler per entry form and asserts the registrations survive, each with a `sideEffects: false` control asserting they are dropped, so the pin cannot pass over a bundler that stopped shaking. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01GTRjn8xBqp75dk7kFupVRt --- .../layout-sideeffects-registration-3899.md | 65 ++++ packages/layout/README.md | 18 + packages/layout/package.json | 6 +- .../__tests__/side-effects-manifest.test.ts | 358 ++++++++++++++++++ 4 files changed, 446 insertions(+), 1 deletion(-) create mode 100644 .changeset/layout-sideeffects-registration-3899.md create mode 100644 packages/layout/src/__tests__/side-effects-manifest.test.ts diff --git a/.changeset/layout-sideeffects-registration-3899.md b/.changeset/layout-sideeffects-registration-3899.md new file mode 100644 index 0000000000..fc84e0a184 --- /dev/null +++ b/.changeset/layout-sideeffects-registration-3899.md @@ -0,0 +1,65 @@ +--- +"@object-ui/layout": patch +--- + +`@object-ui/layout` no longer tells bundlers it has no side effects while registering components at load time (objectui#3899) + +The published manifest declared `"sideEffects": false` — a promise that no module +in the package does anything on evaluation, so any module whose exports go unused +may be dropped whole. But `src/index.ts` ends with a bare +`try { registerLayout(); } catch {}`, and that call is the only thing that puts +`page-header`, `page:card`, `app-shell`, `responsive-grid`, +`navigation-renderer` and `app-schema-renderer` into the `ComponentRegistry`. + +Both statements cannot be true, and a bundler that believes the manifest is +right to delete the registration. Measured with the repo's own bundler by +building `import '@object-ui/layout';` — the side-effect-only import, i.e. the +documented "import it to register" pattern: + +- `sideEffects: false` — the bundle is **0 bytes**. Zero registrations, exit + code 0, no warning. +- after this change — the bundle keeps all six `ComponentRegistry.register` + calls. + +A dropped registration does not fail where it happened. It surfaces later as a +red `Unknown component type` panel (OBJUI-001) on a fully green build, with +nothing in the build log to connect the two. Nobody had been bitten yet only +because every consumer today ALSO imports a named export, which forces the module +to be evaluated regardless — coincidence, not design. objectui#3787 met the +hazard and routed around it by calling `registerLayout()` explicitly. + +`sideEffects` is now the narrowest honest answer: an array naming the modules +that actually register, rather than `true`, which would be honest but would hand +the whole package to every bundler as unshakeable. + +```json +"sideEffects": ["./dist/index.js", "./dist/index.umd.cjs", "./src/index.ts"] +``` + +All three are load-bearing, and the set is derived from the manifest rather than +guessed: + +- `./dist/index.js` and `./dist/index.umd.cjs` are every JS file the manifest's + own entry fields point at (`module` / `main` / `exports` import+require). The + library build inlines everything into those two files, so there is no third + chunk to name. +- `./src/index.ts` is not published (`files` ships `dist` only) but is bundled + for real: `apps/console` and `examples/console-starter` both alias the + specifier straight at `packages/layout/src`, and a bundler reads this same + manifest for those files. With only the published paths declared, the console's + alias shape still produced a 0-byte bundle. + +What deliberately did NOT change: the load-time `registerLayout()` itself. +Replacing it with an explicit registration API is the opposite direction — it +eliminates the side effect instead of declaring it, and it is breaking for any +consumer relying on automatic registration. objectui#3899 leaves that call to the +maintainer, and the two steps do not conflict: once the manifest tells the truth, +the migration to explicit registration can happen whenever it is wanted. + +A new pin (`packages/layout/src/__tests__/side-effects-manifest.test.ts`) runs a +real bundler build per entry form and asserts the registrations survive a +side-effect-only import, with a `sideEffects: false` control per form asserting +they are dropped — so the pin cannot pass because the bundler stopped shaking +anything. The required set is derived from the manifest's own entry fields, so a +renamed build output or a new `exports` subpath fails as a missing declaration +instead of drifting silently. diff --git a/packages/layout/README.md b/packages/layout/README.md index 9e588651e9..b110869f7e 100644 --- a/packages/layout/README.md +++ b/packages/layout/README.md @@ -21,6 +21,24 @@ pnpm add @object-ui/layout - `react-dom` ^18.0.0 || ^19.0.0 - `react-router-dom` ^6.0.0 || ^7.0.0 +## Registration + +Importing this package registers its component keys (`page-header`, `page:card`, +`app-shell`, `responsive-grid`, `navigation-renderer`, `app-schema-renderer`) on +the `ComponentRegistry` as a module load side effect, so the side-effect-only +import is enough: + +```typescript +import '@object-ui/layout'; +``` + +That is a supported entry point, not an accident of the build: `package.json` +declares the registering modules in `sideEffects`, which is what stops a bundler +from tree-shaking a side-effect-only import away (objectui#3899 — the manifest +used to say `"sideEffects": false`, and a bundler honouring it dropped the +registration silently). `registerLayout()` is also exported for hosts that +prefer to register explicitly. + ## Components ### AppShell diff --git a/packages/layout/package.json b/packages/layout/package.json index f68fca7ca5..a43cd02e95 100644 --- a/packages/layout/package.json +++ b/packages/layout/package.json @@ -2,7 +2,11 @@ "name": "@object-ui/layout", "version": "17.3.0", "type": "module", - "sideEffects": false, + "sideEffects": [ + "./dist/index.js", + "./dist/index.umd.cjs", + "./src/index.ts" + ], "main": "dist/index.umd.cjs", "module": "dist/index.js", "types": "dist/index.d.ts", diff --git a/packages/layout/src/__tests__/side-effects-manifest.test.ts b/packages/layout/src/__tests__/side-effects-manifest.test.ts new file mode 100644 index 0000000000..961b59399f --- /dev/null +++ b/packages/layout/src/__tests__/side-effects-manifest.test.ts @@ -0,0 +1,358 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * This package registers its components as a MODULE LOAD SIDE EFFECT, and its + * manifest has to say so (objectui#3899). + * + * `src/index.ts` ends with a bare `try { registerLayout(); } catch {}`, which is + * the only thing that puts `page-header`, `app-shell`, `sidebar-nav`, + * `page:card`, `responsive-grid` and `app-schema-renderer` into the + * `ComponentRegistry`. The manifest used to declare `"sideEffects": false` — + * a promise to bundlers that no module here does anything on evaluation, so any + * module whose exports go unused may be dropped whole. + * + * Both statements cannot be true. Measured on main@230ffd875 by bundling + * `import '@object-ui/layout';` (the side-effect-only import, i.e. the + * documented "import it to register" pattern) with the repo's own bundler: + * + * sideEffects: false -> bundle is 0 bytes, zero registrations + * this array -> bundle keeps all six `ComponentRegistry.register` calls + * + * Zero bytes, exit code 0, no warning. The failure surfaces far away as a red + * `Unknown component type` panel (OBJUI-001) on a green build. Nobody was bitten + * only because every consumer today also imports a NAMED export, which forces + * evaluation regardless — coincidence, not design. objectui#3787 hit the hazard + * and routed around it by calling `registerLayout()` explicitly. + * + * ## What this file pins, and what it deliberately does not + * + * It pins the MANIFEST as a bundling contract: the declaration keeps naming + * every module form a bundler can resolve this package to, and a real bundler + * run confirms each of those forms actually survives a side-effect-only import. + * + * The spelling is NOT what the probes are sensitive to, which is worth writing + * down because it is the natural guess. Measured on vite 8 / rolldown 1.2.1, + * `./dist/index.js`, `dist/index.js`, `dist/*.js` and `**\/index.js` all match + * the same file. So a red probe means the path is not covered AT ALL, or the + * bundler changed how it honours the field — never a leading-`./` nit. + * + * It does NOT pin the auto-registration itself. Replacing it with an explicit + * registration API is a separate, deliberately-not-taken direction (breaking for + * any consumer relying on load-time registration) that the issue leaves to the + * maintainer. If that lands, `declaresLoadTimeRegistration` below turns red — on + * purpose. That test is not defending the side effect, it is defending the + * INVARIANT: the manifest and the module body must agree. Remove the side effect + * and the honest manifest is `false` again, so both change together or neither + * does. A red test here means "update the other half", never "put it back". + */ + +import { describe, it, expect } from 'vitest'; +import { build } from 'vite'; +import { cpSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { dirname, join, resolve } from 'node:path'; +import { tmpdir } from 'node:os'; + +/** `packages/layout` — two levels up from `src/__tests__`. */ +const PKG_DIR = resolve(__dirname, '../..'); +const SRC_DIR = join(PKG_DIR, 'src'); + +interface Manifest { + name: string; + main?: string; + module?: string; + sideEffects?: unknown; + exports?: Record | string>; +} + +const manifest: Manifest = JSON.parse(readFileSync(join(PKG_DIR, 'package.json'), 'utf8')); + +/** `"dist/index.js"` and `"./dist/index.js"` name one file; compare on this. */ +const normalize = (p: string): string => (p.startsWith('./') ? p.slice(2) : p); + +/** + * Every module path a bundler can resolve `@object-ui/layout` to, derived from + * the manifest rather than hardcoded — a renamed `fileName` in `vite.config.ts` + * or a new `exports` subpath then shows up here as a missing declaration instead + * of drifting silently. + * + * `src/index.ts` is in the list even though `files` does not publish it, because + * two in-repo consumers bundle the source tree directly by alias + * (`apps/console/vite.config.ts` and `examples/console-starter/vite.config.ts` + * both map the specifier at `packages/layout/src`), and the bundler reads THIS + * manifest for those files too — measured, not assumed: with the published + * paths alone declared, the console's alias shape still produced a 0-byte + * bundle. Publishing is not the only consumption surface. + */ +function resolvableEntryPaths(): string[] { + const found = new Set(); + for (const field of [manifest.main, manifest.module]) { + if (typeof field === 'string') found.add(normalize(field)); + } + for (const target of Object.values(manifest.exports ?? {})) { + const conditions = typeof target === 'string' ? { default: target } : target; + for (const [condition, file] of Object.entries(conditions)) { + // `types` points at a `.d.ts`; type declarations are erased and never + // carry a side effect, so they are not a bundling surface. + if (condition === 'types' || typeof file !== 'string') continue; + found.add(normalize(file)); + } + } + found.add('src/index.ts'); + return [...found].sort(); +} + +const declaredSideEffects = (): string[] => { + const declared = manifest.sideEffects; + expect( + Array.isArray(declared), + 'packages/layout/package.json must declare `sideEffects` as an ARRAY naming the modules that ' + + 'register components at load time (objectui#3899). `false` is the lie this file exists to catch; ' + + '`true` would be honest but hands the whole package to every bundler as unshakeable.', + ).toBe(true); + return declared as string[]; +}; + +/** The bare specifier the probes resolve, so each probe picks its own target. */ +const SPECIFIER = '@object-ui/layout'; + +/** Marker that only survives if the bundler kept the module body. */ +const MARKER = '__objectui_3899_load_time_side_effect__'; + +/** + * Bundle `import '@object-ui/layout';` — nothing else, no named import — with + * `target` supplying the module, and return the emitted code. + * + * Every bare specifier except the package under test is external, so the bundle + * holds this package's own graph and nothing more. `write: false` keeps it in + * memory; measured at ~250ms cold and ~40ms warm, so this is a real build and + * still cheap enough to be an ordinary unit test. + */ +async function bundleSideEffectOnlyImport(target: string): Promise { + const dir = mkdtempSync(join(tmpdir(), 'objectui-3899-entry-')); + try { + const entry = join(dir, 'entry.mjs'); + writeFileSync(entry, `import ${JSON.stringify(SPECIFIER)};\n`); + + const result: unknown = await build({ + configFile: false, + logLevel: 'silent', + resolve: { alias: { [SPECIFIER]: target } }, + build: { + write: false, + minify: false, + lib: { entry, formats: ['es'], fileName: 'bundle' }, + rollupOptions: { + external: (id: string) => !/^[./]/.test(id) && !id.startsWith('/') && id !== SPECIFIER, + }, + }, + }); + + const bundles = Array.isArray(result) ? result : [result]; + const output = (bundles[0] as { output?: Array<{ type: string; code?: string }> } | undefined)?.output; + if (!output) { + throw new Error( + 'The probe build returned no output. It must produce an in-memory bundle to inspect — a watcher ' + + 'or an empty result means this helper needs updating, not that the assertion below passed.', + ); + } + return output + .filter((chunk) => chunk.type === 'chunk') + .map((chunk) => chunk.code ?? '') + .join('\n'); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +} + +/** + * A copy of this package whose manifest is byte-for-byte the real one except for + * `sideEffects`, used to run the same probe under a different declaration. + * + * Copying rather than mutating the real `package.json`: a test that edits its own + * package manifest leaves a dirty tree behind when it fails, and two of these run + * in the same worker. + * + * `entryPaths` get a marker module written at exactly the manifest's own relative + * paths, which is what lets the published `dist/*` forms be probed in an unbuilt + * worktree. Those probes therefore prove that the DECLARED GLOBS MATCH the + * published paths in the bundler's matcher — not that `dist` holds the + * registration. The latter is what `declaresLoadTimeRegistration` (the source + * side) and the package build (the artifact side) are for. + */ +function mirrorPackage(sideEffects: unknown, entryPaths: string[] = []): string { + const dir = mkdtempSync(join(tmpdir(), 'objectui-3899-mirror-')); + writeFileSync(join(dir, 'package.json'), JSON.stringify({ ...manifest, sideEffects }, null, 2)); + + for (const rel of entryPaths) { + const file = join(dir, normalize(rel)); + mkdirSync(dirname(file), { recursive: true }); + // The marker is a global assignment: a side effect no bundler can prove + // away, so its absence means the MODULE was dropped, never that the + // statement was optimised out. + writeFileSync( + file, + `globalThis[${JSON.stringify(MARKER)}] = ${JSON.stringify(rel)};\n` + + (rel.endsWith('.cjs') ? 'module.exports = { unused: 1 };\n' : 'export const unused = 1;\n'), + ); + } + return dir; +} + +/** The source tree, copied so the same probe can run under a different manifest. */ +function mirrorSourceTree(sideEffects: unknown): string { + const dir = mirrorPackage(sideEffects); + cpSync(SRC_DIR, join(dir, 'src'), { + recursive: true, + filter: (src) => !src.includes('__tests__'), + }); + return join(dir, 'src'); +} + +describe('@object-ui/layout declares its load-time registration (objectui#3899)', () => { + it('names every module form a bundler can resolve, and nothing that is not one', () => { + const declared = declaredSideEffects().map(normalize); + const required = resolvableEntryPaths(); + + const missing = required.filter((p) => !declared.includes(p)); + expect( + missing, + [ + 'A module form of @object-ui/layout is not covered by `sideEffects`, so a bundler resolving the', + 'package that way may drop the registration entirely — silently, on a green build (objectui#3899).', + '', + 'Add each path below to `sideEffects` in packages/layout/package.json, spelled with a leading', + `"./" to match how \`exports\` spells the same paths: ${missing.join(', ')}`, + ].join('\n'), + ).toEqual([]); + + // The other direction. An entry naming nothing real is not harmless: it + // reads as "this module has side effects" to the next author and makes the + // list look maintained when it is not. + const phantom = declared.filter((p) => !required.includes(p)); + expect( + phantom, + [ + '`sideEffects` names a path that is not a module form of this package.', + 'If a NEW module genuinely has load-time side effects, teach resolvableEntryPaths() where it', + 'comes from (an `exports` subpath, a build output, a source alias) so the derivation stays the', + `authority. Otherwise delete it: ${phantom.join(', ')}`, + ].join('\n'), + ).toEqual([]); + + // Anti-vacuity: a derivation that silently produced [] would make both + // assertions above pass over nothing. These are the three forms measured on + // main@230ffd875 — the ESM and UMD published entries plus the source alias. + expect(required).toEqual(['dist/index.js', 'dist/index.umd.cjs', 'src/index.ts']); + }); + + it('declaresLoadTimeRegistration: the barrel still registers on evaluation', () => { + const barrel = readFileSync(join(SRC_DIR, 'index.ts'), 'utf8'); + + // A bare `registerLayout()` call at column 0 — module body, not inside the + // function declaration (whose own body is indented). + expect( + /^\s{0,2}registerLayout\(\);/m.test(barrel), + [ + 'src/index.ts no longer calls `registerLayout()` at load time, so the `sideEffects` array in', + 'package.json is now claiming a side effect that does not happen.', + '', + 'This test does NOT ask for the call back. It asks the two halves to agree: if the registration', + 'became an explicit API the consumer calls (the direction objectui#3899 left to the maintainer),', + 'then the honest manifest is `sideEffects: false` and this file should be deleted with the same', + 'change. What it forbids is exactly one of the two moving.', + ].join('\n'), + ).toBe(true); + }); + + it('keeps a side-effect-only import alive through the workspace source alias', async () => { + // The in-place, real-tree probe: the actual src/, read through the actual + // manifest, in the shape apps/console and examples/console-starter bundle. + const code = await bundleSideEffectOnlyImport(SRC_DIR); + + expect( + code, + 'Bundling `import "@object-ui/layout";` through the workspace src alias dropped the component ' + + 'registrations. This is objectui#3899 exactly: a bundler took the manifest at its word. Check ' + + 'that `sideEffects` names "./src/index.ts".', + ).toContain('page-header'); + + // Every key the barrel registers, so losing all but one cannot pass on the + // strength of that one. Read out of the source rather than listed here: + // objectui#3899's own prose named a `sidebar-nav` key that this package has + // never registered (the SidebarNav component reaches the registry under + // `navigation-renderer`), and a hardcoded list is how that kind of mistake + // becomes a test asserting a component that does not exist. + const registeredKeys = [...readFileSync(join(SRC_DIR, 'index.ts'), 'utf8').matchAll( + /ComponentRegistry\.register\(\s*'([^']+)'/g, + )].map((match) => match[1]); + + expect( + registeredKeys.length, + 'No `ComponentRegistry.register` call found in src/index.ts — the loop below would assert nothing.', + ).toBeGreaterThanOrEqual(6); + + for (const key of registeredKeys) { + expect(code, `the \`${key}\` registration must survive a side-effect-only import`).toContain(key); + } + }); + + it.each(['./dist/index.js', './dist/index.umd.cjs'])( + 'keeps a side-effect-only import alive through the published entry %s', + async (entryPath) => { + const mirror = mirrorPackage(manifest.sideEffects, [entryPath]); + try { + const code = await bundleSideEffectOnlyImport(join(mirror, normalize(entryPath))); + expect( + code, + `The declared \`sideEffects\` globs do not cover ${entryPath}, so a consumer resolving the ` + + 'published package that way loses the registration. Add the path — this is not a spelling ' + + 'nit: rolldown matches it with or without the leading "./", and through `dist/*.js` too.', + ).toContain(MARKER); + } finally { + rmSync(mirror, { recursive: true, force: true }); + } + }, + ); + + it.each(['./dist/index.js', './dist/index.umd.cjs'])( + 'and the declaration is what keeps it: `sideEffects: false` drops %s', + async (entryPath) => { + // The control. Without it every assertion above could be green because the + // bundler never shakes anything here, and the pin would prove nothing — + // the exact shape of "green for an empty reason". This is objectui#3899's + // reverse verification, run every time rather than once by hand. + const mirror = mirrorPackage(false, [entryPath]); + try { + const code = await bundleSideEffectOnlyImport(join(mirror, normalize(entryPath))); + expect( + code, + `A package declaring \`sideEffects: false\` did NOT lose ${entryPath} on a side-effect-only ` + + 'import. The bundler no longer honours the flag the way this pin assumes, so the probes above ' + + 'have stopped measuring anything — fix the probe before trusting them again.', + ).not.toContain(MARKER); + } finally { + rmSync(mirror, { recursive: true, force: true }); + } + }, + ); + + it('and the same control holds for the source tree', async () => { + const mirrored = mirrorSourceTree(false); + try { + const code = await bundleSideEffectOnlyImport(mirrored); + expect( + code, + 'A copy of this package declaring `sideEffects: false` kept its registrations, so the source-alias ' + + 'probe above is not measuring the manifest at all.', + ).not.toContain('page-header'); + } finally { + rmSync(resolve(mirrored, '..'), { recursive: true, force: true }); + } + }); +}); From 8e8a5e2a0c58d2bf86177e30e8757adddd56f829 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 13:32:18 +0000 Subject: [PATCH 2/2] docs(layout): correct the registered keys in the sideEffects pin's header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The file header repeated objectui#3899's own slip — it listed a `sidebar-nav` key this package has never registered. The six real keys are `page-header`, `page:card`, `app-shell`, `responsive-grid`, `navigation-renderer` and `app-schema-renderer`; the assertion already reads them out of `src/index.ts` rather than trusting a written list, which is why the prose was the only place the mistake survived. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01GTRjn8xBqp75dk7kFupVRt --- packages/layout/src/__tests__/side-effects-manifest.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/layout/src/__tests__/side-effects-manifest.test.ts b/packages/layout/src/__tests__/side-effects-manifest.test.ts index 961b59399f..2f728dde4a 100644 --- a/packages/layout/src/__tests__/side-effects-manifest.test.ts +++ b/packages/layout/src/__tests__/side-effects-manifest.test.ts @@ -11,8 +11,8 @@ * manifest has to say so (objectui#3899). * * `src/index.ts` ends with a bare `try { registerLayout(); } catch {}`, which is - * the only thing that puts `page-header`, `app-shell`, `sidebar-nav`, - * `page:card`, `responsive-grid` and `app-schema-renderer` into the + * the only thing that puts `page-header`, `page:card`, `app-shell`, + * `responsive-grid`, `navigation-renderer` and `app-schema-renderer` into the * `ComponentRegistry`. The manifest used to declare `"sideEffects": false` — * a promise to bundlers that no module here does anything on evaluation, so any * module whose exports go unused may be dropped whole.