Skip to content

Commit e342606

Browse files
JoeGermuskaclaude
andcommitted
Fix dev-mode build failures, add CI, commit the missing slide-ID module
`pnpm dev` printed build errors that `pnpm build` never showed, because `pnpm -r build` is topologically ordered while `pnpm -r --parallel dev` discards that ordering: - workers/og-preview is the only workspace member without a source alias, so esbuild resolves core through package.json exports to dist/index.js. Core's watch build emptied dist/ on every rebuild, and the worker failed against the empty window before recovering. Core now only cleans dist/ for a real build. - player's dts pass uses rollupTypes (api-extractor), which intermittently fails on incremental rebuilds. Declarations are now emitted only for a real build — nothing in dev reads them, since every app aliases core and player to src/. While in there: core's `tsc -p tsconfig.build.json` step emitted nothing at all. tsconfig.base.json sets noEmit, which overrode its emitDeclarationOnly, and tsc exits 0 having done nothing — so vite-plugin-dts was the sole emitter and its unscoped include shipped dist/**/*.test.d.ts to npm. Dropped the dead step and its config, scoped the plugin to exclude tests. Also adds `dev:og` for the core/embed/worker subset, so the common case doesn't need all ten dev processes. packages/core/src/adapters/ids.ts and its test were never staged, though 81f1e5f committed every caller — a clean checkout could not build. The new CI workflow builds, typechecks and tests a fresh checkout on push and PR; publish.yml only ran on release tags, which is why this went unnoticed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 81f1e5f commit e342606

9 files changed

Lines changed: 295 additions & 14 deletions

File tree

.github/workflows/ci.yml

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
name: CI
2+
3+
on:
4+
push:
5+
branches: [main]
6+
pull_request:
7+
8+
# A newer push to the same branch or PR makes an in-flight run irrelevant.
9+
concurrency:
10+
group: ci-${{ github.ref }}
11+
cancel-in-progress: true
12+
13+
permissions:
14+
contents: read
15+
16+
jobs:
17+
check:
18+
runs-on: ubuntu-latest
19+
steps:
20+
- uses: actions/checkout@v7
21+
22+
- uses: pnpm/action-setup@v6
23+
with:
24+
version: 11
25+
26+
- uses: actions/setup-node@v7
27+
with:
28+
node-version: 22
29+
cache: pnpm
30+
31+
- run: pnpm install --frozen-lockfile
32+
33+
- run: pnpm typecheck
34+
35+
- run: pnpm test:run
36+
37+
# Last because it is the slowest, but the reason this workflow exists:
38+
# it builds a clean checkout, so a source file that was never committed
39+
# fails here instead of during a release tag.
40+
- run: pnpm build

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
"test": "pnpm -r test",
88
"test:run": "pnpm -r test:run",
99
"dev": "pnpm -r --parallel dev",
10+
"dev:og": "pnpm --parallel --filter @knight-lab/timeline-ng-core --filter @timeline-ng/embed --filter @timeline-ng/og-preview dev",
1011
"authoring": "pnpm --filter @timeline-ng/authoring dev",
1112
"authoring:devnet": "pnpm --filter @timeline-ng/authoring dev:devnet",
1213
"embed": "pnpm --filter @timeline-ng/embed dev",

packages/core/CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/).
2323
and unique — so a timeline-ng file round-trips unchanged — and rebuilds the
2424
rest the same way. Title slides get `title`. Duplicates are suffixed `-2`, `-3`.
2525

26+
### Fixed
27+
28+
- **Declarations for test files no longer ship.** The published tarball included
29+
`dist/**/*.test.d.ts`, because declaration generation ran over all of `src`.
30+
Only the public surface is emitted now.
31+
2632
## [0.4.0] - 2026-08-25
2733

2834
### Added

packages/core/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
"schema"
3030
],
3131
"scripts": {
32-
"build": "vite build && tsc -p tsconfig.build.json && node scripts/schema.mjs",
32+
"build": "vite build && node scripts/schema.mjs",
3333
"build:schema": "node scripts/schema.mjs",
3434
"dev": "vite build --watch",
3535
"test": "vitest",
Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
import { describe, it, expect } from 'vitest';
2+
import { slugifySlideId, isValidSlideId, uniqueSlideId, assignSlideIds } from './ids.ts';
3+
import type { TLTimeline } from '../types.ts';
4+
5+
describe('slugifySlideId', () => {
6+
it('slugs a headline', () => {
7+
expect(slugifySlideId('The Moon Landing')).toBe('the-moon-landing');
8+
});
9+
10+
it('strips HTML and punctuation', () => {
11+
expect(slugifySlideId('<b>Apollo 11:</b> "one small step"')).toBe('apollo-11-one-small-step');
12+
});
13+
14+
it('folds accents rather than dropping the word', () => {
15+
expect(slugifySlideId('Año Nuevo en México')).toBe('ano-nuevo-en-mexico');
16+
});
17+
18+
it('keeps a leading number by prefixing instead of truncating', () => {
19+
expect(slugifySlideId('1969 Moon Landing')).toBe('slide-1969-moon-landing');
20+
});
21+
22+
it('returns empty when nothing usable survives', () => {
23+
expect(slugifySlideId('???')).toBe('');
24+
expect(slugifySlideId('東京')).toBe('');
25+
});
26+
27+
it('produces legal ids', () => {
28+
for (const h of ['The Moon Landing', '1969 Moon Landing', '<b>Apollo 11:</b>']) {
29+
expect(isValidSlideId(slugifySlideId(h))).toBe(true);
30+
}
31+
});
32+
});
33+
34+
describe('assignSlideIds', () => {
35+
it('builds ids from headlines', () => {
36+
const tl: TLTimeline = {
37+
events: [
38+
{ start_date: { year: 2000 }, text: { headline: 'First Thing' } },
39+
{ start_date: { year: 2001 }, text: { headline: 'Second Thing' } },
40+
],
41+
};
42+
assignSlideIds(tl);
43+
expect(tl.events.map(e => e.unique_id)).toEqual(['first-thing', 'second-thing']);
44+
});
45+
46+
it('replaces illegal ids, including the numeric ones legacy imports produced', () => {
47+
const tl: TLTimeline = {
48+
events: [
49+
{ start_date: { year: 2000 }, unique_id: '1', text: { headline: 'First Thing' } },
50+
{ start_date: { year: 2001 }, unique_id: '2', text: { headline: 'Second Thing' } },
51+
],
52+
};
53+
assignSlideIds(tl);
54+
expect(tl.events.map(e => e.unique_id)).toEqual(['first-thing', 'second-thing']);
55+
});
56+
57+
it('keeps ids that are already legal', () => {
58+
const tl: TLTimeline = {
59+
events: [{ start_date: { year: 2000 }, unique_id: 'my-slide', text: { headline: 'First Thing' } }],
60+
};
61+
assignSlideIds(tl);
62+
expect(tl.events[0].unique_id).toBe('my-slide');
63+
});
64+
65+
it('names the title slide "title"', () => {
66+
const tl: TLTimeline = { title: { unique_id: '1', text: { headline: 'My Timeline' } }, events: [] };
67+
assignSlideIds(tl);
68+
expect(tl.title?.unique_id).toBe('title');
69+
});
70+
71+
it('disambiguates repeated headlines', () => {
72+
const tl: TLTimeline = {
73+
events: [
74+
{ start_date: { year: 2000 }, text: { headline: 'Same' } },
75+
{ start_date: { year: 2001 }, text: { headline: 'Same' } },
76+
{ start_date: { year: 2002 }, text: { headline: 'Same' } },
77+
],
78+
};
79+
assignSlideIds(tl);
80+
expect(tl.events.map(e => e.unique_id)).toEqual(['same', 'same-2', 'same-3']);
81+
});
82+
83+
it('does not let a regenerated id collide with a preserved one', () => {
84+
const tl: TLTimeline = {
85+
events: [
86+
{ start_date: { year: 2000 }, unique_id: '1', text: { headline: 'Same' } },
87+
{ start_date: { year: 2001 }, unique_id: 'same', text: { headline: 'Same' } },
88+
],
89+
};
90+
assignSlideIds(tl);
91+
expect(tl.events.map(e => e.unique_id)).toEqual(['same-2', 'same']);
92+
});
93+
94+
it('falls back to a positional id when a headline yields nothing', () => {
95+
const tl: TLTimeline = {
96+
events: [
97+
{ start_date: { year: 2000 } },
98+
{ start_date: { year: 2001 }, text: { headline: '東京' } },
99+
],
100+
};
101+
assignSlideIds(tl);
102+
expect(tl.events.map(e => e.unique_id)).toEqual(['slide-1', 'slide-2']);
103+
for (const e of tl.events) expect(isValidSlideId(e.unique_id!)).toBe(true);
104+
});
105+
106+
it('gives every event an id even with duplicate illegal ids in the source', () => {
107+
const tl: TLTimeline = {
108+
events: [
109+
{ start_date: { year: 2000 }, unique_id: 'dup', text: { headline: 'A' } },
110+
{ start_date: { year: 2001 }, unique_id: 'dup', text: { headline: 'B' } },
111+
],
112+
};
113+
assignSlideIds(tl);
114+
expect(tl.events.map(e => e.unique_id)).toEqual(['dup', 'b']);
115+
});
116+
});
117+
118+
describe('uniqueSlideId', () => {
119+
it('returns the base when it is free', () => {
120+
expect(uniqueSlideId('intro', new Set(['other']))).toBe('intro');
121+
});
122+
123+
it('suffixes past every taken variant', () => {
124+
expect(uniqueSlideId('intro', new Set(['intro', 'intro-2']))).toBe('intro-3');
125+
});
126+
127+
it('does not mutate the set it is given', () => {
128+
const taken = new Set(['intro']);
129+
uniqueSlideId('intro', taken);
130+
expect([...taken]).toEqual(['intro']);
131+
});
132+
});
133+
134+
describe('slugifySlideId with entities', () => {
135+
it('drops HTML entities rather than spelling them out', () => {
136+
expect(slugifySlideId('Rock &amp; Roll')).toBe('rock-roll');
137+
});
138+
});

packages/core/src/adapters/ids.ts

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
import type { TLEvent, TLTimeline } from '../types.ts';
2+
3+
/**
4+
* The shape a slide ID has to have: a letter, then letters, digits, `-` or `_`.
5+
*
6+
* Slide IDs are meant to be usable as URL fragments and CSS/DOM identifiers,
7+
* so the rule is deliberately narrower than "any non-empty string".
8+
*/
9+
export const SLIDE_ID_PATTERN = /^[a-zA-Z][a-zA-Z0-9_-]*$/;
10+
11+
export function isValidSlideId(id: string): boolean {
12+
return SLIDE_ID_PATTERN.test(id);
13+
}
14+
15+
/** Slugs longer than this get cut — long enough to stay readable, short enough to type. */
16+
const MAX_SLUG_LENGTH = 48;
17+
18+
/**
19+
* Turn arbitrary headline text into the longest legal ID it can support.
20+
* Returns '' when nothing usable survives (a headline that is entirely
21+
* punctuation, or entirely in a script with no ASCII transliteration).
22+
*/
23+
export function slugifySlideId(text: string): string {
24+
const slug = text
25+
.replace(/<[^>]*>/g, ' ') // headlines may carry HTML
26+
.replace(/&(?:[a-zA-Z][a-zA-Z0-9]*|#\d+|#x[0-9a-fA-F]+);/g, ' ') // …and entities
27+
.normalize('NFKD')
28+
.replace(/[\u0300-\u036f]/g, '') // é → e, ñ → n
29+
.toLowerCase()
30+
.replace(/[^a-z0-9]+/g, '-')
31+
.replace(/^-+|-+$/g, '')
32+
.slice(0, MAX_SLUG_LENGTH)
33+
.replace(/-+$/, '');
34+
if (!slug) return '';
35+
// A leading digit is illegal, but dropping it would lose real information
36+
// ("1969 Moon Landing"), so prefix instead of truncate.
37+
return /^[a-z]/.test(slug) ? slug : `slide-${slug}`;
38+
}
39+
40+
/**
41+
* `base` if it is free, else `base-2`, `base-3`, … until one is.
42+
* Non-mutating, so callers that only need to *offer* an ID (the editor's
43+
* "generate from the headline" affordance) can use it too.
44+
*/
45+
export function uniqueSlideId(base: string, taken: ReadonlySet<string>): string {
46+
if (!taken.has(base)) return base;
47+
for (let n = 2; ; n++) {
48+
const candidate = `${base}-${n}`;
49+
if (!taken.has(candidate)) return candidate;
50+
}
51+
}
52+
53+
/** Claim `base` in `taken`, appending -2, -3, … until it is free. */
54+
function claim(base: string, taken: Set<string>): string {
55+
const id = uniqueSlideId(base, taken);
56+
taken.add(id);
57+
return id;
58+
}
59+
60+
/**
61+
* Give every event in an imported timeline a legal, unique slide ID, in place.
62+
*
63+
* IDs already present in the source are kept when they are legal and unique —
64+
* an author who set `unique_id` deliberately, or a timeline-ng file being read
65+
* back in, should round-trip unchanged. Everything else is regenerated from the
66+
* headline, which is what makes an imported timeline's IDs worth looking at:
67+
* TimelineJS 3's spreadsheet format has no ID column at all, so a sheet import
68+
* would otherwise arrive as 1..N, which is both meaningless and illegal.
69+
*/
70+
export function assignSlideIds(timeline: TLTimeline): void {
71+
const all: TLEvent[] = timeline.title ? [timeline.title, ...timeline.events] : timeline.events;
72+
const taken = new Set<string>();
73+
74+
for (const event of all) {
75+
const id = event.unique_id;
76+
if (id && isValidSlideId(id) && !taken.has(id)) taken.add(id);
77+
else delete event.unique_id;
78+
}
79+
80+
all.forEach((event, i) => {
81+
if (event.unique_id) return;
82+
const isTitle = event === timeline.title;
83+
const base = (isTitle ? 'title' : slugifySlideId(event.text?.headline ?? '')) || `slide-${i + 1}`;
84+
event.unique_id = claim(base, taken);
85+
});
86+
}

packages/core/tsconfig.build.json

Lines changed: 0 additions & 10 deletions
This file was deleted.

packages/core/vite.config.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,26 @@
11
import { defineConfig } from 'vite';
22
import dts from 'vite-plugin-dts';
33

4+
// `pnpm dev` runs this as `vite build --watch`. Nothing in dev reads dist/ —
5+
// every app aliases this package to src/ — so declarations are pure cost there,
6+
// and clearing dist/ between rebuilds actively breaks workers/og-preview, which
7+
// resolves us through package.json exports rather than an alias.
8+
const watching = process.argv.includes('--watch') || process.argv.includes('-w');
9+
410
export default defineConfig({
511
plugins: [
6-
dts({ include: ['src'], insertTypesEntry: true }),
12+
// This is the only thing that emits declarations for this package; the
13+
// `tsc -p tsconfig.build.json` step the build script used to run was a
14+
// silent no-op, because tsconfig.base.json's `noEmit` overrode its
15+
// `emitDeclarationOnly`. Tests need excluding here or they ship.
16+
...(watching ? [] : [dts({
17+
include: ['src'],
18+
exclude: ['src/**/*.test.ts'],
19+
insertTypesEntry: true,
20+
})]),
721
],
822
build: {
23+
emptyOutDir: !watching,
924
lib: {
1025
entry: 'src/index.ts',
1126
formats: ['es', 'cjs'],

packages/player/vite.config.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,11 @@ import { svelte } from '@sveltejs/vite-plugin-svelte';
33
import dts from 'vite-plugin-dts';
44
import { resolve } from 'path';
55

6+
// `pnpm dev` runs this as `vite build --watch`. Nothing in dev reads dist/*.d.ts
7+
// — every app aliases this package to src/ — and rollupTypes (api-extractor)
8+
// intermittently fails on incremental rebuilds, so only emit types for a real build.
9+
const watching = process.argv.includes('--watch') || process.argv.includes('-w');
10+
611
export default defineConfig({
712
resolve: {
813
alias: {
@@ -12,7 +17,7 @@ export default defineConfig({
1217
},
1318
plugins: [
1419
svelte(),
15-
dts({
20+
...(watching ? [] : [dts({
1621
include: ['src'],
1722
insertTypesEntry: true,
1823
rollupTypes: true,
@@ -23,7 +28,7 @@ export default defineConfig({
2328
`from '@knight-lab/timeline-ng-core'`
2429
),
2530
}),
26-
}),
31+
})]),
2732
],
2833
build: {
2934
lib: {

0 commit comments

Comments
 (0)