Skip to content

chore(style-types): migrate to per-package dist/ build pipeline#963

Closed
layershifter wants to merge 2 commits into
microsoft:mainfrom
layershifter:chore/workspace-migration-phase-2-style-types
Closed

chore(style-types): migrate to per-package dist/ build pipeline#963
layershifter wants to merge 2 commits into
microsoft:mainfrom
layershifter:chore/workspace-migration-phase-2-style-types

Conversation

@layershifter
Copy link
Copy Markdown
Member

Summary

Phase 2 of the workspaces + project-references migration (stacks on top of #962). Picks the smallest leaf package and replaces the workspace-rooted @nx/js:tsc build with a per-package tsc + asset-copy pipeline.

  • Build output: packages/style-types/dist/ (was dist/packages/style-types).
  • New tools/copy-pkg-assets.mjs copies package.json (stripped of scripts + devDependencies), README.md, and LICENSE.md into the per-package dist/.
  • package.json gets a published-style exports map: @griffel/source (no-op today, forward-compat with the customConditions in later phases), types pointing at src/index.ts so consumers can resolve types without first building, default at dist/index.js for runtime, and a files: ["dist/", README, LICENSE, CHANGELOG] field.
  • Drop @griffel/style-types from tsconfig.base.json paths — consumers resolve via the yarn workspace symlink + the package.json above.
  • .gitignore: drop the leading slash from dist so per-package dist/ directories are also ignored.

Test plan

  • yarn nx run-many --target=type-check --all — 20/20 pass
  • yarn nx run-many --target=build --all — 15/15 pass
  • yarn nx run-many --target=test --all — 16/16 pass (excluding the pre-existing e2e/rspack flake)
  • yarn check-dependencies — clean

Stack

This PR builds on #962 — the diff above #962 is the style-types-only delta. Don't merge until #962 lands.

🤖 Generated with Claude Code

layershifter and others added 2 commits May 21, 2026 17:48
First slice of the larger workspaces + project-references migration.
This one only touches private packages — the published @griffel/*
packages keep their path aliases in tsconfig.base.json untouched.

- e2e/eslint, e2e/typescript: add @griffel/e2e-utils as devDependency
  (workspace:*). Both already import from @griffel/e2e-utils; the
  resolution was implicit via the path alias.
- e2e/rspack: tighten the existing "*" e2e-utils dep to workspace:*
  for consistency.
- Drop @griffel/e2e-utils and @griffel/update-shorthands aliases from
  tsconfig.base.json — no consumer needs them now that the deps are
  declared explicitly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Second slice of the workspaces migration. Now picks up the smallest
leaf package and replaces the workspace-rooted @nx/js:tsc build with
per-package tsc + asset copy.

- Build output: packages/style-types/dist/ (was dist/packages/style-types).
- New tools/copy-pkg-assets.mjs copies package.json (stripped of scripts +
  devDependencies), README, and LICENSE into the per-package dist/.
- package.json gets a published-style exports map: @griffel/source (no-op
  today but forward-compat with the customConditions in later phases),
  types pointing at src/index.ts so consumers can resolve types
  without first building, default at dist/index.js for runtime, and a
  files: ["dist/", README, LICENSE, CHANGELOG] field.
- Drop @griffel/style-types from tsconfig.base.json paths — consumers
  now resolve via the yarn workspace symlink + the package.json above.
- .gitignore: drop the leading slash from "dist" so per-package dist/
  directories are also ignored.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@layershifter layershifter requested a review from a team as a code owner May 21, 2026 16:14
@github-actions
Copy link
Copy Markdown
Contributor

📊 Bundle size report

✅ No changes found

@layershifter
Copy link
Copy Markdown
Member Author

Code review

Found 1 issue:

  1. Published tarball will ship a broken types resolution. The outer packages/style-types/package.json declares "types": "./src/index.ts" (and the @griffel/source / types export conditions also point at ./src/index.ts), but files only includes dist/, README.md, LICENSE.md, CHANGELOG.md. Beachball publishes from the package root using the outer package.json, so the tarball will not contain any src/. Verified locally with npm pack --dry-run: the tarball contains dist/index.d.ts but no src/, while the included package.json still points types at ./src/index.ts. Every downstream consumer will get "cannot find type definitions" once this ships.

"sideEffects": false,
"main": "./dist/index.js",
"types": "./src/index.ts",
"exports": {
".": {
"@griffel/source": "./src/index.ts",
"types": "./src/index.ts",
"default": "./dist/index.js"
},
"./package.json": "./package.json"
},
"files": [
"dist/",
"README.md",
"LICENSE.md",
"CHANGELOG.md"
],
"dependencies": {

Fix options: point types and the types / non-source export conditions at the emitted ./dist/index.d.ts (declarations are already produced by tsconfig.lib.json), or add src/ to files if the goal is to keep ship-the-source semantics for @griffel/source consumers. The dist/package.json written by tools/copy-pkg-assets.mjs is also not used at publish time (npm reads the outer one), so fixing the outer file is what matters.

Other things considered but not flagged: the dead dist/package.json (harmless because beachball doesn't publish from dist/), the @griffel/update-shorthands path-mapping removal (no callers found), the .gitignore /distdist change (no tracked dist/* files), and the cwd vs documented usage of copy-pkg-assets.mjs (functionally works since the script resolves against repoRoot).

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@layershifter
Copy link
Copy Markdown
Member Author

Slicing the migration into phases doesn't pan out — closing in favor of the single migration in #774.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant