From 9d8a59ecf3f462c2e7a16de85af791588e43cded Mon Sep 17 00:00:00 2001 From: hexplus Date: Mon, 7 Sep 2026 03:08:31 -0600 Subject: [PATCH 1/9] docs(changelog): scope the 4.4.0 entry to the patterns addition --- CHANGELOG.md | 48 +++++++++++ README.md | 13 ++- cdn.ts | 34 +++++++- examples/chess/chess-island.js | 130 ++++++++++++++++++++++++++-- examples/chess/chess.css | 140 ++++++++++++++++++++++++++----- examples/chess/index.html | 149 ++++++++++++++++----------------- package-lock.json | 4 +- package.json | 2 +- tests/dist-artifacts.test.ts | 81 ++++++++++++++++++ tsup.cdn.config.ts | 14 +++- 10 files changed, 498 insertions(+), 117 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b36da93..5c3bbc9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,54 @@ This project follows [Semantic Versioning](https://semver.org/). --- --- +## [4.4.0] — 2026-09-07 + +### Added — `patterns` on the CDN bundle + +`cdn.ts` re-exported `./index` and nothing else, which left the no-build story +quietly incomplete. Islands are the feature most often reached for WITHOUT a +bundler — one script tag, server HTML, done — but an island that wanted +`machine` could not have it: it lives in the `sibujs/patterns` entry point, and +a ` + +``` + +Load **one or the other**, never both. It is a separate file rather than a +merge so the default bundle keeps its byte budget: patterns costs ~13% gzip, +and most pages never call any of it. `cdn.full.dev.global.js` is the +development counterpart, and both are reachable as `sibujs/cdn-full` and +`sibujs/cdn-full-dev`. A bundled app needs neither file — import from `"sibujs"` and let your bundler define `__SIBU_DEV__` per build (the Vite and webpack plugins in diff --git a/cdn.full.ts b/cdn.full.ts new file mode 100644 index 0000000..c16ec04 --- /dev/null +++ b/cdn.full.ts @@ -0,0 +1,40 @@ +// --------------------------------------------------------------------------- +// Sibu — CDN / IIFE bundle, core + patterns +// +// The same runtime as `cdn.global.js` with the `sibujs/patterns` entry point +// merged in. Load ONE of the two, never both. +// +// Usage: +// +// +// +// WHY THIS IS A SEPARATE FILE. A diff --git a/package.json b/package.json index ac26deb..1d24cf6 100644 --- a/package.json +++ b/package.json @@ -133,6 +133,12 @@ }, "./cdn-dev": { "default": "./dist/cdn.dev.global.js" + }, + "./cdn-full": { + "default": "./dist/cdn.full.global.js" + }, + "./cdn-full-dev": { + "default": "./dist/cdn.full.dev.global.js" } }, "publishConfig": { diff --git a/src/patterns/contracts.ts b/src/patterns/contracts.ts index 6acb732..e0d8817 100644 --- a/src/patterns/contracts.ts +++ b/src/patterns/contracts.ts @@ -1,9 +1,18 @@ /** * Runtime prop validation and strict typing contracts for SibuJS. * Provides runtime type checking for component props in development mode. + * + * Both diagnostics here gate on `DEV`, the same foldable flag the rest of the + * framework uses, so a `__SIBU_DEV__: false` define removes them and their + * message text entirely. They used to test `process.env.NODE_ENV` directly, + * which is not a `define` target and does not exist in a browser at all — so + * every browser build treated itself as development: `validateProps` ran and + * warned, `assertType` threw, and the strings shipped. That went unnoticed + * while these functions were reachable only through a bundler; putting + * `patterns` on the CDN made them a production artifact. */ -declare var process: { env?: { NODE_ENV?: string } } | undefined; +import { DEV, devWarn } from "../core/dev"; // ─── Type Validators ──────────────────────────────────────────────────────── @@ -91,13 +100,12 @@ export type PropSchema = { /** * Validate props against a schema. Returns validated props with defaults applied. - * In production mode (process.env.NODE_ENV === 'production'), validation is skipped - * and only defaults are applied for performance. + * In production builds validation is skipped and only defaults are applied, so the + * returned value is the same either way — only the checking disappears. */ export function validateProps(props: Partial, schema: PropSchema): Props { const result = { ...props } as Record; const errors: string[] = []; - const isDev = typeof process === "undefined" || process?.env?.NODE_ENV !== "production"; for (const [key, def] of Object.entries(schema)) { const propDef: PropDef = typeof def === "function" ? { type: def as Validator } : (def as PropDef); @@ -107,7 +115,7 @@ export function validateProps(props: Partial, schem result[key] = typeof propDef.default === "function" ? (propDef.default as () => unknown)() : propDef.default; } - if (!isDev) continue; // Skip validation in production + if (!DEV) continue; // folds to `continue`, dropping every check below // Check required if (propDef.required && result[key] == null) { @@ -130,8 +138,11 @@ export function validateProps(props: Partial, schem } } - if (errors.length > 0 && isDev) { - console.warn(`[SibuJS] Prop validation errors:\n${errors.map((e) => ` - ${e}`).join("\n")}`); + // `DEV &&` comes first so the template literal sits inside the folded branch. + // A bare `devWarn(...)` would still evaluate its argument, leaving the message + // text in the bundle even though it could never print. + if (DEV && errors.length > 0) { + devWarn(`Prop validation errors:\n${errors.map((e) => ` - ${e}`).join("\n")}`); } return result as Props; @@ -158,10 +169,12 @@ export function defineStrictComponent(config: { /** * Assert that a value satisfies a contract at runtime. - * No-op in production builds. + * + * No-op in production builds — genuinely, now. The previous guard returned early + * only when `process` existed, so in a browser it fell through and threw. */ export function assertType(value: unknown, validator: Validator, label?: string): asserts value is T { - if (typeof process !== "undefined" && process?.env?.NODE_ENV === "production") return; + if (!DEV) return; const result = validator(value as T, label || "value"); if (result !== true) { throw new TypeError(`[SibuJS Contract] ${result}`); diff --git a/tests/dist-artifacts.test.ts b/tests/dist-artifacts.test.ts index 42d0647..6edec07 100644 --- a/tests/dist-artifacts.test.ts +++ b/tests/dist-artifacts.test.ts @@ -29,6 +29,8 @@ import { describe, expect, it } from "vitest"; const ROOT = resolve(__dirname, ".."); const PROD_CDN = resolve(ROOT, "dist/cdn.global.js"); const DEV_CDN = resolve(ROOT, "dist/cdn.dev.global.js"); +const FULL_CDN = resolve(ROOT, "dist/cdn.full.global.js"); +const FULL_DEV_CDN = resolve(ROOT, "dist/cdn.full.dev.global.js"); // Every diagnostic, identified by a literal only it emits, which survives // minification verbatim. Keep in step with the marker list in @@ -48,6 +50,15 @@ const DIAGNOSTIC_MARKERS = { "setup rejection explanation": "the promise returned by the setup also rejected", } as const; +// Diagnostics that live in `sibujs/patterns`, so they only appear in the +// bundles that carry it. `validateProps` gated on `process.env.NODE_ENV` +// until 4.4.0 — not a `define` target, and absent in a browser, so the check +// ran and warned in every browser build. Nothing caught it because this list +// is hand-maintained; the behavioural tests below are the real guard. +const PATTERNS_DIAGNOSTIC_MARKERS = { + "prop validation errors": "Prop validation errors", +} as const; + // `dist/` only exists after `npm run build`. Skipping locally keeps a plain // `vitest` run working on a fresh clone; on CI a missing artifact is a failure, // because CI always builds first and a silent skip there would hide exactly the @@ -147,45 +158,121 @@ function loadCdnGlobal(file: string): Record { return context.Sibu as Record; } -describe.skipIf(!built && !onCI)("the core CDN global", () => { +const fullBuilt = existsSync(FULL_CDN) && existsSync(FULL_DEV_CDN); + +describe.skipIf(!built && !onCI)("the default CDN global", () => { it("self-registers an object on window", () => { expect(typeof loadCdnGlobal(PROD_CDN)).toBe("object"); }); - it("carries the patterns helpers a no-build island cannot otherwise reach", () => { + it("carries core and nothing else", () => { + // This is the file every no-build page downloads, so anything merged in is + // paid for by consumers who never call it. `patterns` cost +13% gzip on its + // own, which is why it ships as `cdn.full.global.js` instead. const Sibu = loadCdnGlobal(PROD_CDN); - expect(typeof Sibu.machine).toBe("function"); - expect(typeof (Sibu.patterns as Record).machine).toBe("function"); + expect(typeof Sibu.signal).toBe("function"); + expect(Sibu.machine).toBeUndefined(); + expect(Sibu.patterns).toBeUndefined(); + expect(Sibu.ui).toBeUndefined(); + expect(Sibu.createDialogAria).toBeUndefined(); }); - it("does NOT carry the ui behaviour layer", () => { - // The boundary, asserted from the core side: merging `sibujs/ui` in would - // make every no-build consumer download forms, virtual lists and - // transitions to get `signal`. - const Sibu = loadCdnGlobal(PROD_CDN); - expect(Sibu.createDialogAria).toBeUndefined(); - expect(Sibu.createFocusManager).toBeUndefined(); - expect(Sibu.ui).toBeUndefined(); + it("stays within its byte budget", () => { + // 80,202 B raw / 26,330 B gzip was the size before patterns was merged in + // and then split back out. The default bundle must not drift above it + // without someone deciding to; a review caught exactly that drift once. + expect(statSync(PROD_CDN).size).toBeLessThanOrEqual(80_202); + }); +}); + +describe.skipIf(!fullBuilt && !onCI)("the core + patterns CDN global", () => { + it("both full bundles exist (run `npm run build` first)", () => { + expect(existsSync(FULL_CDN), `missing ${FULL_CDN}`).toBe(true); + expect(existsSync(FULL_DEV_CDN), `missing ${FULL_DEV_CDN}`).toBe(true); }); - it("keeps `dialog` and `form` as the element tag factories", async () => { - // `sibujs/ui` exports its own `dialog` and `form`, and they are NOT the tag - // factories of the same name. Keeping the bundles apart is what stops that - // ambiguity reaching `Sibu`; if the two are ever merged, this fails. - // + it("carries the patterns surface a no-build page cannot otherwise reach", () => { + const Sibu = loadCdnGlobal(FULL_CDN); + expect(typeof Sibu.machine).toBe("function"); + expect(typeof (Sibu.patterns as Record).machine).toBe("function"); + expect(typeof Sibu.signal).toBe("function"); + }); + + it("lets core win every name collision", async () => { // Identified by arity rather than by calling them: a tag factory needs a // DOM, and identity comparison is meaningless across a separate bundle. // Minification preserves parameter count. - const Sibu = loadCdnGlobal(PROD_CDN); + const Sibu = loadCdnGlobal(FULL_CDN); const core = (await import("../dist/index.js")) as unknown as Record void>; const ui = (await import("../dist/ui.js")) as unknown as Record void>; // The premise this test rests on, asserted rather than assumed. expect(ui.dialog).not.toBe(core.dialog); expect(core.dialog.length).not.toBe(ui.dialog.length); - expect(core.form.length).not.toBe(ui.form.length); expect((Sibu.dialog as () => void).length).toBe(core.dialog.length); expect((Sibu.form as () => void).length).toBe(core.form.length); }); + + it("compiles the patterns diagnostics out of production", () => { + const prod = readFileSync(FULL_CDN, "utf8"); + const dev = readFileSync(FULL_DEV_CDN, "utf8"); + for (const [name, marker] of Object.entries(PATTERNS_DIAGNOSTIC_MARKERS)) { + expect(prod, `${name} survived into the production bundle`).not.toContain(marker); + expect(dev, `${name} is missing from the development bundle`).toContain(marker); + } + }); + + // The tests that actually matter: the marker list above is hand-maintained + // and missed this for a whole release. These run the published bytes. + it("validateProps neither validates nor warns in production", () => { + const warnings: string[] = []; + const context = createContext({ + console: { warn: (...a: unknown[]) => warnings.push(a.join(" ")), error() {}, log() {} }, + }) as Record; + context.window = context; + runInContext(readFileSync(FULL_CDN, "utf8"), context); + const Sibu = context.Sibu as Record; + const validators = Sibu.validators as unknown as Record; + + const out = (Sibu.validateProps as unknown as (p: object, s: object) => Record)( + { n: "not a number" }, + { n: { type: validators.number, required: true } }, + ); + + expect(warnings).toEqual([]); + // Defaults still applied, value untouched: only the checking disappears. + expect(out.n).toBe("not a number"); + }); + + it("validateProps does warn in the development bundle", () => { + // The negative above is only meaningful if the positive holds. + const warnings: string[] = []; + const context = createContext({ + console: { warn: (...a: unknown[]) => warnings.push(a.join(" ")), error() {}, log() {} }, + }) as Record; + context.window = context; + runInContext(readFileSync(FULL_DEV_CDN, "utf8"), context); + const Sibu = context.Sibu as Record; + const validators = Sibu.validators as unknown as Record; + + (Sibu.validateProps as unknown as (p: object, s: object) => unknown)( + { n: "not a number" }, + { n: { type: validators.number, required: true } }, + ); + + expect(warnings.join(" ")).toContain("Prop validation errors"); + }); + + it("assertType is a no-op in production and throws in development", () => { + const call = (file: string) => { + const Sibu = loadCdnGlobal(file); + const validators = Sibu.validators as Record; + (Sibu.assertType as (v: unknown, val: unknown, l?: string) => void)("nope", validators.number, "n"); + }; + // It guarded on `process.env.NODE_ENV`, which does not exist in a browser, + // so the early return never fired and this threw on every CDN page. + expect(() => call(FULL_CDN)).not.toThrow(); + expect(() => call(FULL_DEV_CDN)).toThrow(/Contract/); + }); }); diff --git a/tests/example-chess-smoke.test.ts b/tests/example-chess-smoke.test.ts index d19bc7d..462a7e7 100644 --- a/tests/example-chess-smoke.test.ts +++ b/tests/example-chess-smoke.test.ts @@ -125,11 +125,30 @@ describe.skipIf(!distBuilt || !vendorBuilt)("chess example — production output } expect(failures).toEqual([]); - // The graph really was walked: the island, the vendored engine and the - // package's own entry points. - expect(seen.size).toBeGreaterThan(3); + // The graph really was walked: the island and the vendored engine. + expect(seen.size).toBeGreaterThan(1); expect([...seen].some((u) => u.endsWith("/vendor/chess.js"))).toBe(true); - expect([...seen].some((u) => u.includes("/dist/index.js"))).toBe(true); + + // And the framework is NOT in it. The example takes SibuJS from the + // + + diff --git a/src/patterns/contracts.ts b/src/patterns/contracts.ts index 583e12e..0b57f30 100644 --- a/src/patterns/contracts.ts +++ b/src/patterns/contracts.ts @@ -112,51 +112,45 @@ export type PropSchema = { // ─── validateProps ────────────────────────────────────────────────────────── /** - * Validate props against a schema. Returns validated props with defaults applied. - * In production builds validation is skipped and only defaults are applied, so the - * returned value is the same either way — only the checking disappears. - */ -/** - * Normalize one schema entry and apply its default, returning the normalized - * definition. + * Validate props against a schema, returning the props with defaults applied. * - * A module-level function rather than a closure inside `validateProps`: a - * closure would be allocated on every call, which is the cost this whole - * arrangement exists to avoid. + * Defaults are production behaviour; the checking is development-only and is + * compiled out of production builds. The two are not interchangeable when the + * schema's callbacks touch outside state: a validator that runs in development + * and not in production can leave a later property's default factory reading + * different state. Keep defaults and validators free of side effects if the + * two modes must agree exactly. * - * @param result - The props object being built, mutated in place. - * @param key - The property being settled. - * @param def - Its schema entry, in either the shorthand or object form. - * @returns The normalized definition, for the caller to validate against. + * @param props - The props to validate. Not mutated. + * @param schema - Per-property definitions, in either the shorthand + * (a bare {@link Validator}) or object form. + * @returns A new object: the props, plus any defaults that applied. */ -function applyDefault(result: Record, key: string, def: unknown): PropDef { - const propDef: PropDef = typeof def === "function" ? { type: def as Validator } : (def as PropDef); - if (result[key] == null && propDef.default !== undefined) { - result[key] = typeof propDef.default === "function" ? (propDef.default as () => unknown)() : propDef.default; - } - return propDef; -} - export function validateProps(props: Partial, schema: PropSchema): Props { const result = { ...props } as Record; - // TWO LOOPS, one per mode — not one loop with a guard inside it, and not two - // passes over the same schema. + // TWO WHOLE LOOPS, one per mode, with no shared helper between them. // - // Ordering is observable. Defaults and validators are user callbacks that may - // read or write outside state, so each property must be finished — normalize, - // default, validate — before the next one starts. Running every default and - // then every validator reorders those calls: a later property's factory would - // observe state an earlier property's validator had not yet written. + // Ordering is observable: defaults and validators are user callbacks that may + // read or write outside state, so each property is finished — normalize, + // default, validate — before the next begins. Running all defaults and then + // all validators would reorder those calls. // - // The `errors` array must be declared INSIDE the guarded branch. Hoisted - // above a shared loop it outlived the fold as a dead `[]`, allocated on every - // production call and never read. + // Everything development-only lives inside the branch, INCLUDING the `errors` + // array and the normalization. Both leaked out of production once: the array + // by being hoisted above a shared loop, the normalization by living in a + // helper both branches called. A shared helper is also where an allocation + // hides from a test that reads `validateProps.toString()`, which is why the + // duplication below is deliberate. if (typeof __SIBU_DEV__ !== "undefined" ? __SIBU_DEV__ : DEV) { const errors: string[] = []; for (const [key, def] of Object.entries(schema)) { - const propDef = applyDefault(result, key, def); + const propDef: PropDef = typeof def === "function" ? { type: def as Validator } : (def as PropDef); + + if (result[key] == null && propDef.default !== undefined) { + result[key] = typeof propDef.default === "function" ? (propDef.default as () => unknown)() : propDef.default; + } if (propDef.required && result[key] == null) { errors.push(`Prop '${key}' is required`); @@ -183,8 +177,19 @@ export function validateProps(props: Partial, schem return result as Props; } - // Production: defaults only, in the same order, allocating nothing extra. - for (const [key, def] of Object.entries(schema)) applyDefault(result, key, def); + // Production: defaults only, in the same order, allocating nothing. + for (const [key, def] of Object.entries(schema)) { + // The shorthand form IS a bare validator, so it carries no default and + // there is nothing to do. Normalizing it to `{ type: def }` would allocate + // an object per entry for validation that does not run here. + if (typeof def === "function") continue; + + const fallback = (def as PropDef).default; + if (result[key] == null && fallback !== undefined) { + result[key] = typeof fallback === "function" ? (fallback as () => unknown)() : fallback; + } + } + return result as Props; } diff --git a/tests-browser/chess.spec.ts b/tests-browser/chess.spec.ts index b3cd8e2..d999807 100644 --- a/tests-browser/chess.spec.ts +++ b/tests-browser/chess.spec.ts @@ -37,16 +37,20 @@ const TO_PROMOTION: Array<[string, string]> = [ // through green. Every request for the CDN tag is answered with the local // build, which is the code under test. // The spec is ESM, so `__dirname` does not exist here. -const LOCAL_CDN = resolve(dirname(fileURLToPath(import.meta.url)), "..", "dist", "cdn.global.js"); +const DIST = resolve(dirname(fileURLToPath(import.meta.url)), "..", "dist"); test.beforeEach(async ({ page }) => { - await page.route("**unpkg.com/**/cdn.global.js", (route) => + // Matches whichever CDN artifact the page asks for and answers with the + // local build of the same name, so switching the example's '; + + it("finds a live classic script", () => { + const live = `${island}`; + expect(classicScriptSources(live)).toContain(FULL_CDN_SRC); + expect(scriptTagIndex(live, FULL_CDN_SRC)).toBeGreaterThan(-1); + }); + + it("ignores a script commented out across several lines", () => { + const multiline = ` + + ${island} +`; + expect(classicScriptSources(multiline)).not.toContain(FULL_CDN_SRC); + expect(scriptTagIndex(multiline, FULL_CDN_SRC)).toBe(-1); + }); + + it("ignores a script commented out on one line", () => { + const singleLine = ` + + ${island} +`; + expect(classicScriptSources(singleLine)).not.toContain(FULL_CDN_SRC); + expect(scriptTagIndex(singleLine, FULL_CDN_SRC)).toBe(-1); + }); + + it("ignores a filename mentioned in ordinary comment prose", () => { + const prose = `${island}`; + expect(classicScriptSources(prose)).toEqual([]); + expect(scriptTagIndex(prose, FULL_CDN_SRC)).toBe(-1); + }); + + it("excludes module scripts from the classic list", () => { + expect(classicScriptSources(island)).toEqual([]); + }); + + it("detects a live core-only tag, which the page assertion then rejects", () => { + const core = `${island}`; + expect(classicScriptSources(core)).toEqual([CORE_CDN_SRC]); + }); + + it("orders a live runtime tag before the island, and a commented one not at all", () => { + const live = `${island}`; + expect(scriptTagIndex(live, FULL_CDN_SRC)).toBeLessThan(scriptTagIndex(live, "./chess-island.js")); + + const commented = `${island}`; + expect(scriptTagIndex(commented, FULL_CDN_SRC)).toBe(-1); + }); +}); + describe.skipIf(!distBuilt || !vendorBuilt)("chess example — production output is servable", () => { it("serves the directory URL as the example page", async () => { // The classic deployment failure: `/examples/chess/` resolving to a @@ -137,37 +246,6 @@ describe.skipIf(!distBuilt || !vendorBuilt)("chess example — production output expect([...seen].some((u) => u.includes("/dist/"))).toBe(false); }, 30_000); - /** - * The `src` of every CLASSIC script in a document, in source order. - * - * Attribute-level, not substring: `html.indexOf("cdn.global.js")` matched the - * explanatory HTML comment above the tag and reported a passing test while - * the page loaded a different artifact entirely. A filename mentioned in - * prose can no longer satisfy anything here. - * - * Module scripts are excluded so the runtime tag and the island can be told - * apart, and so `type="module"` written on the CDN tag would fail rather than - * quietly change its loading semantics. - */ - function classicScriptSources(html: string): string[] { - return [...html.matchAll(/]*)>/gi)] - .filter((match) => !/\btype\s*=\s*["']module["']/i.test(match[1])) - .map((match) => match[1].match(/\bsrc\s*=\s*["']([^"']+)["']/i)?.[1]) - .filter((src): src is string => src !== undefined); - } - - /** The index of the first script tag whose `src` is exactly `src`. */ - function scriptTagIndex(html: string, src: string): number { - for (const match of html.matchAll(/]*>/gi)) { - const attr = match[0].match(/\bsrc\s*=\s*["']([^"']+)["']/i)?.[1]; - if (attr === src) return match.index ?? -1; - } - return -1; - } - - const FULL_CDN_SRC = "https://unpkg.com/sibujs@latest/dist/cdn.full.global.js"; - const CORE_CDN_SRC = "https://unpkg.com/sibujs@latest/dist/cdn.global.js"; - it("loads the FULL runtime bundle from a real script tag, not the core-only one", async () => { const html = await (await fetch(`${BASE}/examples/chess/index.html`)).text(); const sources = classicScriptSources(html);