🧹 refactor: cut the duplicated checks and prose from the tree-shaking work
Acting on ponytail-review. The findings were about verification written twice
and comments restating the CHANGELOG, not about the fixes themselves.
- dist/cjs/package.json had two writers: the build script echoed it, then
build-bundle.mjs rewrote it three lines later with the sideEffects entry.
One writer now, the one that knows what belongs in it.
- Dropped the banner-version assert. Same process, same `pkg.version` going in
and coming out — the "stale bundle" it claimed to catch cannot happen.
- Dropped the `includes('toInstanceSync')` text check. ci.yml imports the flat
bundle and typeof-checks the export, which is the same claim actually tested.
- Dropped the treeshake case asserting annotations survive minification; the
build already asserts it. Its one non-duplicated assertion was the floor on
the expected count, and that gap was real: the build compared flat >= esm, so
if tsc ever stopped emitting annotations both sides would read 0 and the
assert would pass on nothing. Folded in as an explicit `expected < 30` check,
verified by stripping the annotations and watching the build fail.
- Removed the `CEREALE` placeholder from treeshake.test.ts. A template language
for one variable, with two no-op `.replace('CEREALE', 'unused')` calls left
behind by it. Interpolated directly.
- Removed `.replace(/\.js$/, '.js')`, which was the identity function.
- Trimmed the comment above the `Symbol.metadata` install from 16 lines to 7,
and the one above `minifySyntax` from 9 to 3, keeping the parts that are not
written down anywhere else.
Also synced the CHANGELOG's byte table to the README's. The two had already
diverged in the webpack column — which is the duplication the review warned
about, showing up before anyone edited either on purpose.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SAcqrz3FcadkYr3xG32CjK
This commit is contained in:
+4
-4
@@ -45,10 +45,10 @@ Measured, minified, across esbuild / rollup / webpack:
|
|||||||
| --- | ---: | ---: | ---: |
|
| --- | ---: | ---: | ---: |
|
||||||
| `flattenErrors` | 394 | 367 | 394 |
|
| `flattenErrors` | 394 | 367 | 394 |
|
||||||
| one decorator | 1,837 | 1,823 | 1,818 |
|
| one decorator | 1,837 | 1,823 | 1,818 |
|
||||||
| `validateSync` | 3,722 | 3,554 | 3,823 |
|
| `validateSync` | 3,722 | 3,554 | 3,738 |
|
||||||
| `toPlainSync` | 7,744 | 7,769 | 7,832 |
|
| `toPlainSync` | 7,744 | 7,769 | 7,771 |
|
||||||
| `toInstanceSync` | 7,900 | 7,942 | 7,956 |
|
| `toInstanceSync` | 7,900 | 7,942 | 7,944 |
|
||||||
| a typical DTO | 10,395 | 10,402 | 10,372 |
|
| a typical DTO | 10,395 | 10,402 | 10,360 |
|
||||||
| everything | 26,266 | 25,671 | 26,879 |
|
| everything | 26,266 | 25,671 | 26,879 |
|
||||||
|
|
||||||
The serializer and deserializer drop independently. The validator is kept by both mapping
|
The serializer and deserializer drop independently. The validator is kept by both mapping
|
||||||
|
|||||||
+1
-1
@@ -38,7 +38,7 @@
|
|||||||
"!src/example.ts"
|
"!src/example.ts"
|
||||||
],
|
],
|
||||||
"scripts": {
|
"scripts": {
|
||||||
"build": "rm -rf dist && tsc -p tsconfig.cjs.json && tsc -p tsconfig.esm.json && echo '{\"type\": \"commonjs\"}' > dist/cjs/package.json && node scripts/build-bundle.mjs",
|
"build": "rm -rf dist && tsc -p tsconfig.cjs.json && tsc -p tsconfig.esm.json && node scripts/build-bundle.mjs",
|
||||||
"build:docs": "node scripts/build-docs.mjs",
|
"build:docs": "node scripts/build-docs.mjs",
|
||||||
"demo": "node --no-warnings=ExperimentalWarning --loader ts-node/esm src/example.ts",
|
"demo": "node --no-warnings=ExperimentalWarning --loader ts-node/esm src/example.ts",
|
||||||
"type-check": "tsc --noEmit",
|
"type-check": "tsc --noEmit",
|
||||||
|
|||||||
+16
-23
@@ -31,15 +31,9 @@ await build({
|
|||||||
bundle: true,
|
bundle: true,
|
||||||
format: 'esm',
|
format: 'esm',
|
||||||
target: 'esnext',
|
target: 'esnext',
|
||||||
// NOT `minify: true`. That turns on `minifyWhitespace`, which strips comments — including
|
// NOT `minify: true`: `minifyWhitespace` strips comments, /*#__PURE__*/ included, and a
|
||||||
// the /*#__PURE__*/ annotations that make the rules droppable. A bundler fed the fully
|
// bundler fed the result keeps every unused rule — 5,066 bytes for one decorator against
|
||||||
// minified file re-inherits the exact bug those annotations fixed: one decorator came out
|
// 1,837. Asserted below. Costs about a kilobyte gzipped.
|
||||||
// at 5,066 bytes against 1,837 from the per-module entry, with all 26 unrelated rule
|
|
||||||
// messages back in the output.
|
|
||||||
//
|
|
||||||
// Syntax and identifier minification keep them. The cost is 34.6 KB raw against 26.0 KB,
|
|
||||||
// but 9.8 KB gzipped against 8.7 KB — about a kilobyte over the wire, which is a fair price
|
|
||||||
// for a file that behaves correctly however someone ends up using it.
|
|
||||||
minifySyntax: true,
|
minifySyntax: true,
|
||||||
minifyIdentifiers: true,
|
minifyIdentifiers: true,
|
||||||
sourcemap: true,
|
sourcemap: true,
|
||||||
@@ -52,22 +46,21 @@ await build({
|
|||||||
outfile,
|
outfile,
|
||||||
});
|
});
|
||||||
|
|
||||||
// The banner names a version, so a stale bundle would misreport itself rather than merely be
|
|
||||||
// out of date. Cheap to assert, and the build is the only place that can.
|
|
||||||
const emitted = await readFile(outfile, 'utf8');
|
|
||||||
if (!emitted.includes(`cereale ${pkg.version}`)) {
|
|
||||||
throw new Error('the bundle banner does not carry the current version');
|
|
||||||
}
|
|
||||||
for (const name of ['toInstanceSync', 'IsString']) {
|
|
||||||
if (!emitted.includes(name)) throw new Error(`${name} is missing from the flat bundle`);
|
|
||||||
}
|
|
||||||
|
|
||||||
// The whole reason this file is not fully minified. Asserting it here means a future change to
|
// The whole reason this file is not fully minified. Asserting it here means a future change to
|
||||||
// the minify options fails the build rather than silently tripling what a bundler keeps.
|
// the minify options fails the build rather than silently tripling what a bundler keeps.
|
||||||
const annotations = (emitted.match(/__PURE__/g) ?? []).length;
|
//
|
||||||
const expected = (await readFile(path.join(dist, 'esm/decorators.js'), 'utf8').then(
|
// The floor on `expected` is not decoration: comparing the two counts alone passes vacuously if
|
||||||
(s) => (s.match(/__PURE__/g) ?? []).length
|
// tsc ever stops emitting the annotations, since 0 >= 0. It has to be wrong in both directions.
|
||||||
));
|
const count = (s) => (s.match(/__PURE__/g) ?? []).length;
|
||||||
|
const emitted = await readFile(outfile, 'utf8');
|
||||||
|
const annotations = count(emitted);
|
||||||
|
const expected = count(await readFile(path.join(dist, 'esm/decorators.js'), 'utf8'));
|
||||||
|
if (expected < 30) {
|
||||||
|
throw new Error(
|
||||||
|
`dist/esm/decorators.js carries only ${expected} /*#__PURE__*/ annotations; src/decorators.ts ` +
|
||||||
|
'writes 30. The compiler is dropping them, so every consumer keeps all 68 rules.'
|
||||||
|
);
|
||||||
|
}
|
||||||
if (annotations < expected) {
|
if (annotations < expected) {
|
||||||
throw new Error(
|
throw new Error(
|
||||||
`the flat bundle kept ${annotations} /*#__PURE__*/ annotations but dist/esm/decorators.js has ` +
|
`the flat bundle kept ${annotations} /*#__PURE__*/ annotations but dist/esm/decorators.js has ` +
|
||||||
|
|||||||
+6
-15
@@ -15,22 +15,13 @@ import type { ClassConstructor } from './interfaces.js';
|
|||||||
*/
|
*/
|
||||||
const METADATA_KEY: symbol = (Symbol as { metadata?: symbol }).metadata ?? Symbol.for('Symbol.metadata');
|
const METADATA_KEY: symbol = (Symbol as { metadata?: symbol }).metadata ?? Symbol.for('Symbol.metadata');
|
||||||
|
|
||||||
// Also installed globally, because a consumer's own compiler emit reads `Symbol.metadata`
|
// Also installed globally: tsc's decorator emit reads `Symbol.metadata` directly rather than
|
||||||
// directly and does not share our fallback. tsc emits
|
// falling back the way we do — `typeof Symbol === "function" && Symbol.metadata ? … : void 0` —
|
||||||
|
// so without this a decorated class gets `metadata: undefined` and no rules at all.
|
||||||
//
|
//
|
||||||
// const _metadata = typeof Symbol === "function" && Symbol.metadata ? Object.create(null) : void 0;
|
// `sideEffects` in package.json keeps the statement through bundling, and must name `index.*`
|
||||||
//
|
// as well as this module: marking only this one leaves the barrel droppable, so the edge to it
|
||||||
// so on a runtime without the well-known symbol the class is decorated with `metadata:
|
// is pruned before this marking is ever read. Pinned by treeshake.test.ts.
|
||||||
// undefined` and ends up with no metadata at all. (esbuild's `__knownSymbol` has the same
|
|
||||||
// `Symbol.for` fallback we do and needs nothing from us; tsc does.)
|
|
||||||
//
|
|
||||||
// `sideEffects` in package.json is what keeps this statement through bundling — and it has to
|
|
||||||
// name `index.ts`/`index.js` as well as this module. Marking only this one is not enough: the
|
|
||||||
// barrel is then itself side-effect-free, so a bundler drops the `export * from './metadata.js'`
|
|
||||||
// edge before this module's own marking is ever consulted, and the install silently vanishes.
|
|
||||||
// Measured on `import { configure } from 'cereale'`: absent from all three of esbuild, webpack
|
|
||||||
// and rollup until the barrel was listed too. It costs ~100 bytes, and only for imports that
|
|
||||||
// pull in nothing else — every entry point that touches a model was already byte-identical.
|
|
||||||
((Symbol as { metadata?: symbol }).metadata as symbol | undefined) ??= METADATA_KEY;
|
((Symbol as { metadata?: symbol }).metadata as symbol | undefined) ??= METADATA_KEY;
|
||||||
|
|
||||||
export interface ValidationArguments {
|
export interface ValidationArguments {
|
||||||
|
|||||||
+12
-21
@@ -1,7 +1,7 @@
|
|||||||
import { describe, it, expect } from 'vitest';
|
import { describe, it, expect } from 'vitest';
|
||||||
import { build } from 'esbuild';
|
import { build } from 'esbuild';
|
||||||
import { mkdtemp, rm, writeFile } from 'node:fs/promises';
|
import { mkdtemp, rm, writeFile } from 'node:fs/promises';
|
||||||
import { existsSync, readFileSync } from 'node:fs';
|
import { existsSync } from 'node:fs';
|
||||||
import { tmpdir } from 'node:os';
|
import { tmpdir } from 'node:os';
|
||||||
import path from 'node:path';
|
import path from 'node:path';
|
||||||
|
|
||||||
@@ -33,13 +33,13 @@ const MARKER = {
|
|||||||
naming: 'SCREAMING_SNAKE_CASE',
|
naming: 'SCREAMING_SNAKE_CASE',
|
||||||
} as const;
|
} as const;
|
||||||
|
|
||||||
const ENTRY = path.resolve('src/index.js').replace(/\.js$/, '.js');
|
const ENTRY = JSON.stringify(path.resolve('src/index.js'));
|
||||||
|
|
||||||
async function bundle(source: string): Promise<string> {
|
async function bundle(source: string): Promise<string> {
|
||||||
const dir = await mkdtemp(path.join(tmpdir(), 'cereale-shake-'));
|
const dir = await mkdtemp(path.join(tmpdir(), 'cereale-shake-'));
|
||||||
try {
|
try {
|
||||||
const entry = path.join(dir, 'entry.ts');
|
const entry = path.join(dir, 'entry.ts');
|
||||||
await writeFile(entry, source.replace('CEREALE', JSON.stringify(ENTRY)));
|
await writeFile(entry, source);
|
||||||
const result = await build({
|
const result = await build({
|
||||||
entryPoints: [entry],
|
entryPoints: [entry],
|
||||||
bundle: true,
|
bundle: true,
|
||||||
@@ -75,7 +75,7 @@ function expectShaken(code: string, keeps: string[], drops: string[]) {
|
|||||||
describe('tree-shaking', () => {
|
describe('tree-shaking', () => {
|
||||||
it('drops the 67 rules you did not import', async () => {
|
it('drops the 67 rules you did not import', async () => {
|
||||||
const code = await bundle(`
|
const code = await bundle(`
|
||||||
import { IsString } from CEREALE;
|
import { IsString } from ${ENTRY};
|
||||||
export const d = IsString();
|
export const d = IsString();
|
||||||
`);
|
`);
|
||||||
|
|
||||||
@@ -90,7 +90,7 @@ describe('tree-shaking', () => {
|
|||||||
|
|
||||||
it('keeps the deserializer and drops the serializer when only reading', async () => {
|
it('keeps the deserializer and drops the serializer when only reading', async () => {
|
||||||
const code = await bundle(`
|
const code = await bundle(`
|
||||||
import { toInstanceSync } from CEREALE;
|
import { toInstanceSync } from ${ENTRY};
|
||||||
export const f = (C, p) => toInstanceSync(C, p, { validate: false });
|
export const f = (C, p) => toInstanceSync(C, p, { validate: false });
|
||||||
`);
|
`);
|
||||||
|
|
||||||
@@ -101,7 +101,7 @@ describe('tree-shaking', () => {
|
|||||||
|
|
||||||
it('keeps the serializer and drops the deserializer when only writing', async () => {
|
it('keeps the serializer and drops the deserializer when only writing', async () => {
|
||||||
const code = await bundle(`
|
const code = await bundle(`
|
||||||
import { toPlainSync } from CEREALE;
|
import { toPlainSync } from ${ENTRY};
|
||||||
export const f = (o) => toPlainSync(o, { validate: false });
|
export const f = (o) => toPlainSync(o, { validate: false });
|
||||||
`);
|
`);
|
||||||
|
|
||||||
@@ -110,7 +110,7 @@ describe('tree-shaking', () => {
|
|||||||
|
|
||||||
it('drops both engines when only validating', async () => {
|
it('drops both engines when only validating', async () => {
|
||||||
const code = await bundle(`
|
const code = await bundle(`
|
||||||
import { validateSync } from CEREALE;
|
import { validateSync } from ${ENTRY};
|
||||||
export const f = (o) => validateSync(o);
|
export const f = (o) => validateSync(o);
|
||||||
`);
|
`);
|
||||||
|
|
||||||
@@ -130,7 +130,7 @@ describe('tree-shaking', () => {
|
|||||||
*/
|
*/
|
||||||
it('installs Symbol.metadata even when nothing model-shaped is imported', async () => {
|
it('installs Symbol.metadata even when nothing model-shaped is imported', async () => {
|
||||||
const code = await bundle(`
|
const code = await bundle(`
|
||||||
import { configure } from CEREALE;
|
import { configure } from ${ENTRY};
|
||||||
export const f = (o) => configure(o);
|
export const f = (o) => configure(o);
|
||||||
`);
|
`);
|
||||||
|
|
||||||
@@ -140,7 +140,7 @@ describe('tree-shaking', () => {
|
|||||||
|
|
||||||
it('costs almost nothing to import only an error helper', async () => {
|
it('costs almost nothing to import only an error helper', async () => {
|
||||||
const code = await bundle(`
|
const code = await bundle(`
|
||||||
import { flattenErrors } from CEREALE;
|
import { flattenErrors } from ${ENTRY};
|
||||||
export const f = (e) => flattenErrors(e);
|
export const f = (e) => flattenErrors(e);
|
||||||
`);
|
`);
|
||||||
|
|
||||||
@@ -150,7 +150,7 @@ describe('tree-shaking', () => {
|
|||||||
|
|
||||||
it('still contains everything when everything is used', async () => {
|
it('still contains everything when everything is used', async () => {
|
||||||
const code = await bundle(`
|
const code = await bundle(`
|
||||||
import * as cereale from CEREALE;
|
import * as cereale from ${ENTRY};
|
||||||
export default cereale;
|
export default cereale;
|
||||||
`);
|
`);
|
||||||
|
|
||||||
@@ -175,7 +175,7 @@ describe('tree-shaking', () => {
|
|||||||
const code = await bundle(`
|
const code = await bundle(`
|
||||||
import { IsString } from ${JSON.stringify(path.join(dist, 'cereale.min.js'))};
|
import { IsString } from ${JSON.stringify(path.join(dist, 'cereale.min.js'))};
|
||||||
export const d = IsString();
|
export const d = IsString();
|
||||||
`.replace('CEREALE', 'unused'));
|
`);
|
||||||
|
|
||||||
expectShaken(code, [MARKER.isString], [MARKER.isLatitude, MARKER.isSemVer, MARKER.serializer]);
|
expectShaken(code, [MARKER.isString], [MARKER.isLatitude, MARKER.isSemVer, MARKER.serializer]);
|
||||||
expect(code.length).toBeLessThan(3000);
|
expect(code.length).toBeLessThan(3000);
|
||||||
@@ -191,20 +191,11 @@ describe('tree-shaking', () => {
|
|||||||
const code = await bundle(`
|
const code = await bundle(`
|
||||||
import { configure } from ${JSON.stringify(path.join(dist, 'esm/index.js'))};
|
import { configure } from ${JSON.stringify(path.join(dist, 'esm/index.js'))};
|
||||||
export const f = (o) => configure(o);
|
export const f = (o) => configure(o);
|
||||||
`.replace('CEREALE', 'unused'));
|
`);
|
||||||
|
|
||||||
expect(code, 'the Symbol.metadata install was pruned from dist/esm').toContain('Symbol.metadata');
|
expect(code, 'the Symbol.metadata install was pruned from dist/esm').toContain('Symbol.metadata');
|
||||||
expectShaken(code, [], [MARKER.isString, MARKER.serializer, MARKER.deserializer]);
|
expectShaken(code, [], [MARKER.isString, MARKER.serializer, MARKER.deserializer]);
|
||||||
});
|
});
|
||||||
|
|
||||||
it.runIf(built)('keeps its purity annotations through minification', async () => {
|
|
||||||
const flat = readFileSync(path.join(dist, 'cereale.min.js'), 'utf8');
|
|
||||||
const perModule = readFileSync(path.join(dist, 'esm/decorators.js'), 'utf8');
|
|
||||||
const count = (s: string) => (s.match(/__PURE__/g) ?? []).length;
|
|
||||||
|
|
||||||
expect(count(perModule), 'src annotations should reach dist/esm').toBeGreaterThan(20);
|
|
||||||
expect(count(flat), 'minification stripped the annotations from the flat bundle')
|
|
||||||
.toBeGreaterThanOrEqual(count(perModule));
|
|
||||||
});
|
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user