Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .changeset/7292-turbo-owns-build-order.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
---
---

Build orchestration only; no published behaviour changes.

`@object-ui/components` and `@object-ui/site` each carried a hand-written
`prebuild` (and, for components, a `pretest` pointing at it) that listed the
workspace packages their build needed. That list is a second, unchecked copy of
the package's own `dependencies`, and the components one had drifted: it named
three packages while the closure turbo derives is seven, so
`pnpm --filter @object-ui/components build` failed inside `@object-ui/react` on
any tree where `@object-ui/i18n` and `@object-ui/data-objectstack` were not
already built. Both hooks are deleted; turbo's `build.dependsOn: ["^build"]` is
now the single source of build order, as it already was for every path CI takes.

Trade-off, recorded here rather than left implicit: the bare per-package forms
`pnpm --filter @object-ui/components build` and `pnpm --filter @object-ui/components test`
no longer build their upstream packages first. They were never a supported entry
point on a clean tree — that is the defect this removes, not a capability it
takes away — but on a warm tree they used to be self-sufficient and now are not.
The supported forms are `turbo run build --filter=@object-ui/components` and
`turbo run test --filter=@object-ui/components` (turbo's `test` task also
declares `dependsOn: ["^build"]`), plus the repo-root `pnpm test`. Every
in-repo document and hint string that taught the direct form now teaches the
turbo form, and the root `site:build` script goes through turbo, matching what
CI already ran.
1 change: 0 additions & 1 deletion apps/site/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
"version": "3.1.0",
"private": true,
"scripts": {
"prebuild": "pnpm --filter @object-ui/example-schema-catalog build",
"build": "next build",
"lint": "eslint .",
"dev": "next dev",
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
"test:integration": "vitest run --project dom --project dom-heavy",
"test:dist": "turbo run test:dist --filter=@object-ui/components",
"site:dev": "pnpm --filter @object-ui/site dev",
"site:build": "pnpm --filter @object-ui/site build",
"site:build": "turbo run build --filter=@object-ui/site",
"site:start": "pnpm --filter @object-ui/site start",
"test:watch": "vitest",
"test:ui": "vitest --ui",
Expand Down
17 changes: 11 additions & 6 deletions packages/components/README_SHADCN_SYNC.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,14 @@ If you don't have network access or prefer manual control:

7. **Test the Component**
```bash
pnpm --filter @object-ui/components build
pnpm --filter @object-ui/components test
turbo run build --filter=@object-ui/components
turbo run test --filter=@object-ui/components
```

Go through turbo, not `pnpm --filter @object-ui/components build`: turbo
derives this package's build closure from its `dependencies`, and on a tree
where those are not built yet the bare per-package build fails.

## Using Official Shadcn CLI

You can also use the official Shadcn CLI:
Expand Down Expand Up @@ -241,13 +245,14 @@ className="... dark:bg-background/95 dark:backdrop-blur-sm"

```bash
# Type check
pnpm --filter @object-ui/components type-check
turbo run type-check --filter=@object-ui/components

# Build
pnpm --filter @object-ui/components build
# Build (turbo builds the upstream packages this one needs; a bare
# `pnpm --filter @object-ui/components build` does not)
turbo run build --filter=@object-ui/components

# Run tests
pnpm --filter @object-ui/components test
turbo run test --filter=@object-ui/components

# Integration test
pnpm test
Expand Down
2 changes: 0 additions & 2 deletions packages/components/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@
},
"scripts": {
"build": "vite build && node scripts/build-css.mjs",
"prebuild": "pnpm --filter @object-ui/types build && pnpm --filter @object-ui/core build && pnpm --filter @object-ui/react build",
"pretest": "pnpm run prebuild",
"test": "vitest run --root ../.. packages/components/",
"test:dist": "OBJECTUI_DIST_PINS=1 vitest run --root ../.. --config vitest.config.mts --project dist",
"type-check": "tsc --noEmit && tsc -p tsconfig.test.json",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,8 @@ describe('page:header — BUILT artifact (objectui#7183, re-derived from PR obje
'meant to be reached through `pnpm test:dist`, whose turbo task carries ' +
'`dependsOn: ["build"]` for the package under test and therefore builds it first. ' +
'Running the project directly builds nothing — build it yourself with ' +
'`pnpm --filter @object-ui/components build`.',
'`turbo run build --filter=@object-ui/components`, which orders the packages ' +
'this one is built from.',
).toBe(true);
});

Expand Down
16 changes: 10 additions & 6 deletions scripts/__tests__/helpers/build-program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,17 @@ import { rel, repoRoot, type WorkspacePackage } from './turbo-inputs';
* plugins). Declared explicitly, and empty, so the shared walker's
* key-directed designation rule is visibly a decision here.
* - A `pnpm --filter <pkg> build` SEGMENT IS A TASK-GRAPH FACT, NOT AN INPUTS
* FACT. Two packages have a `prebuild` that builds another workspace package
* (pnpm runs `prebuild` as part of `pnpm run build`, so turbo runs it too).
* The right answer to "this package's build needs that package's dist" is
* turbo's `dependsOn: ["^build"]`, not a `$TURBO_ROOT$` glob over another
* FACT. The right answer to "this package's build needs that package's dist"
* is turbo's `dependsOn: ["^build"]`, not a `$TURBO_ROOT$` glob over another
* package's source. So delegations are collected and returned rather than
* walked, and `../turbo-build-inputs.test.ts` asserts each delegated package
* is a DECLARED dependency — which is what makes `^build` order it.
* walked. NO PACKAGE DELEGATES TODAY (objectui#7292 deleted the last two:
* `packages/components` hand-listed types/core/react in a `prebuild` while
* its real closure was seven packages, and `apps/site` named the schema
* catalog it already declares as a dependency), and
* `../turbo-build-inputs.test.ts` now RATCHETS that to zero rather than
* policing the survivors: a hand-written list of a package's own build
* closure is a second, unchecked copy of its `dependencies`, and the
* collected `delegations` are what makes re-introducing one fail loudly.
*/

/** Vite's config candidates, in its own precedence order. */
Expand Down
54 changes: 25 additions & 29 deletions scripts/__tests__/turbo-build-inputs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,38 +172,34 @@ describe('turbo `build` inputs cover every out-of-package file (objectui#4185)',
/**
* A `pnpm --filter <pkg> build` segment inside a `prebuild` is a task-GRAPH
* fact, not an inputs fact: the right answer to "this build needs that
* package's dist" is `dependsOn: ["^build"]`, which turbo already declares —
* but `^build` only orders DECLARED dependencies. Two packages delegate this
* way (`apps/site` to `@object-ui/example-schema-catalog`, `packages/components`
* to types/core/react), and the derivation deliberately does not walk into
* them. That narrowing is sound only while each delegated package is a real
* dependency, so assert it rather than assume it.
* package's dist" is `dependsOn: ["^build"]`, which turbo already declares
* and derives from the dependency graph. A hand-written chain is a SECOND,
* unchecked copy of the package's own `dependencies` — and it drifts:
* objectui#7292 measured `packages/components`' three-package `prebuild`
* against the seven-package closure turbo computes, and a clean worktree's
* `pnpm --filter @object-ui/components build` died inside `@object-ui/react`
* on the two packages the list had never gained.
*
* The predecessor of this assertion policed the survivors (each delegated
* package must be a declared dependency). That is the weaker guard: a
* delegation whose target IS declared still duplicates the graph, still
* drifts, and still fails only on the trees nobody looks at. So this ratchets
* the class to zero instead. The derivation still COLLECTS delegations —
* that is what makes re-introducing one fail here rather than silently widen
* an unswept build program.
*/
it('every delegated build target is a declared dependency', () => {
it('no build lifecycle script delegates to another package with `pnpm --filter`', () => {
const delegating = DERIVED.filter((pkg) => pkg.program.delegations.length > 0);
expect(
delegating.map((pkg) => pkg.name),
'no package delegates a build any more — drop this assertion with the narrowing it ' +
'defends, in helpers/build-program.ts',
).not.toHaveLength(0);

for (const pkg of delegating) {
const manifest = JSON.parse(
fs.readFileSync(path.join(pkg.dir, 'package.json'), 'utf8'),
) as { dependencies?: Record<string, string>; devDependencies?: Record<string, string> };
const declared = new Set([
...Object.keys(manifest.dependencies ?? {}),
...Object.keys(manifest.devDependencies ?? {}),
]);
const undeclared = pkg.program.delegations.filter((name) => !declared.has(name));
expect(
undeclared,
`${pkg.name} builds ${undeclared.join(', ')} through a \`pnpm --filter\` lifecycle ` +
`script without declaring ${undeclared.length === 1 ? 'it' : 'them'} as a dependency, ` +
`so turbo's \`dependsOn: ["^build"]\` does not order the two. Declare the dependency, ` +
`or teach build-program.ts to walk into the delegated package's program.`,
).toEqual([]);
}
delegating.map((pkg) => `${pkg.name} -> ${pkg.program.delegations.join(', ')}`),
'a build lifecycle script builds another workspace package through `pnpm --filter`. ' +
"That hand-writes a copy of this package's build closure next to the `dependencies` " +
'turbo already derives it from, and the two drift silently — the copy is only ever ' +
'exercised on a tree where the missing packages happen to be built already. Declare ' +
"the dependency in package.json and let turbo's `dependsOn: [\"^build\"]` order it; " +
'build the package with `turbo run build --filter=<pkg>`, not `pnpm --filter <pkg> ' +
'build`.',
).toEqual([]);
});

/**
Expand Down
2 changes: 1 addition & 1 deletion scripts/build-plugin-stylesheet.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ export function createPluginStylesheetBuilder({ postcss, tailwind }) {
"correctly via the `build` task's `dependsOn: [\"^build\"]`; a bare single-package",
'build does not.',
'',
' pnpm --filter @object-ui/components build',
' turbo run build --filter=@object-ui/components',
].join('\n'),
);
}
Expand Down
2 changes: 1 addition & 1 deletion scripts/shadcn-sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -912,7 +912,7 @@ async function updateAllComponents(options = {}) {
log('2. Fix or roll back — the type-check above failed.', 'dim');
} else {
log('2. Test the components: pnpm test', 'dim');
log('3. Build the package: pnpm --filter @object-ui/components build', 'dim');
log('3. Build the package: turbo run build --filter=@object-ui/components', 'dim');
}
}

Expand Down
Loading