Uh oh!
There was an error while loading. Please reload this page.
📦 Tighten publish tarballs: restrict files to dist - #216
Conversation
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughStandardizes npm publication across workspace packages by adding or tightening ChangesPackage publication allowlists
Estimated code review effort: 2 (Simple) | ~12 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
@effectionx/bdd@effectionx/chain@effectionx/context-api@effectionx/converge@effectionx/effect-ts@effectionx/fetch@effectionx/fs@effectionx/fx@effectionx/inline@effectionx/jsonl-store@effectionx/middleware@effectionx/node@effectionx/process@effectionx/raf@effectionx/scope-eval@effectionx/signals@effectionx/state-api@effectionx/stream-helpers@effectionx/stream-yaml@effectionx/task-buffer@effectionx/test-adapter@effectionx/timebox@effectionx/tinyexec@effectionx/vitest@effectionx/watch@effectionx/websocket@effectionx/workercommit: |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
state-api/package.json (1)
5-12:⚠️ Potential issue | 🟡 MinorPre-existing:
keywordsuses non-approved terms.None of
effection,effectionx,state,reducer,middleware,reactiveare in the approved keyword set.reactivein particular looks like a typo forreactivity, which would be valid. Since you're already touching every package's manifest in this PR, this may be a cheap fix to land alongside.As per coding guidelines (
.policies/package-json-metadata.md): "keywords (array of approved category keywords: testing, io, process, streams, concurrency, reactivity, interop, platform; at least one; no custom keywords)".🧹 Suggested change
- "keywords": [- "effection",- "effectionx",- "state",- "reducer",- "middleware",- "reactive"- ],+ "keywords": ["reactivity"],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@state-api/package.json` around lines 5 - 12, The package.json "keywords" array contains non-approved terms; update the "keywords" field to use only approved category keywords (testing, io, process, streams, concurrency, reactivity, interop, platform), ensuring at least one is present; specifically replace "reactive" with "reactivity" if intended and remove custom entries like "effection", "effectionx", "state", "reducer", and "middleware" so the final "keywords" array in package.json contains only allowed keywords (e.g., ["reactivity"] or another approved set appropriate for this package).inline/package.json (1)
44-48:⚠️ Potential issue | 🟡 MinorRisk:
prepackonly builds the wasm; the TS→JSdist/outputs (dist/mod.js,dist/esbuild.js) aren't guaranteed fresh at pack time.With
files: ["dist", ...wasm], publishing relies ondist/already being built. Theprepackhook here only runs the cargo/wasm build — it doesn't trigger the TypeScript compilation that producesdist/mod.jsanddist/esbuild.jsreferenced bymain/types/exports. If publishing goes through a pipeline that always runspnpm buildfirst this is fine, but a directnpm publish/npm packin this package would ship a stale or missingdist/with a freshly rebuilt wasm.Consider chaining the TS build into
prepack(e.g.,"prepack": "pnpm run build && pnpm run build:bundle") or documenting that this package must only be published via the monorepo's release script.#!/bin/bash# Confirm how dist/ is produced for inline and whether release tooling guarantees build before pack.set -e fd -HI '^package.json$' inline 2>/dev/null --exec cat echo'--- root build script and release pipeline ---' cat package.json 2>/dev/null ||true fd -HI -t f . .github/workflows 2>/dev/null --exec sh -c 'echo "## {}"; cat {}'||true rg -n --hidden -g '!**/node_modules/**''inline' .github 2>/dev/null ||true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inline/package.json` around lines 44 - 48, The prepack script only runs "build:bundle" (the Cargo/wasm build) so published packages can contain stale/missing JS outputs under dist/ (dist/mod.js, dist/esbuild.js); update the package.json "prepack" entry to also run the TypeScript build (e.g., invoke the existing JS build script before build:bundle) or document/ensure the monorepo release pipeline always runs the root/package-level build first; specifically modify the "prepack" script key to chain the TS->JS build (the script that produces dist/) with "build:bundle" so the files array entries referencing dist/ are freshly produced at pack time.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bdd/package.json`:
- Line 29: The package.json "files" whitelist is missing development source
files referenced by the exports map (the "." and "./node" development
conditions), so update the "files" array to include "mod.ts" and "mod.node.ts"
in addition to the existing entries; ensure the "files" field includes ["dist",
"mod.ts", "mod.node.ts", "mod.deno.ts"] so the development export targets used
by the exports map (for "." and "./node") are published.
In `@inline/package.json`:
- Line 48: The "files" entry currently hard-codes
"swc/target/wasm32-wasip1/release/swc_plugin_inline.wasm" which is brittle; add
a package.json prepack (or prepare) script that asserts the wasm artifact exists
(e.g., run a small node script or shell check that globs for swc/**/*.wasm or
the expected artifact names and exits non‑zero if none found) and fail the pack
if missing, and optionally replace the single hard-coded entry with a stable
pattern (e.g., include the directory "swc" or "swc/**/*.wasm" in the "files"
array) so changes to target triple/profile/crate name won’t silently drop the
artifact; reference the "files" array entry and add a "prepack" script key to
package.json that runs the existence check.
---
Outside diff comments:
In `@inline/package.json`:
- Around line 44-48: The prepack script only runs "build:bundle" (the Cargo/wasm
build) so published packages can contain stale/missing JS outputs under dist/
(dist/mod.js, dist/esbuild.js); update the package.json "prepack" entry to also
run the TypeScript build (e.g., invoke the existing JS build script before
build:bundle) or document/ensure the monorepo release pipeline always runs the
root/package-level build first; specifically modify the "prepack" script key to
chain the TS->JS build (the script that produces dist/) with "build:bundle" so
the files array entries referencing dist/ are freshly produced at pack time.
In `@state-api/package.json`:
- Around line 5-12: The package.json "keywords" array contains non-approved
terms; update the "keywords" field to use only approved category keywords
(testing, io, process, streams, concurrency, reactivity, interop, platform),
ensuring at least one is present; specifically replace "reactive" with
"reactivity" if intended and remove custom entries like "effection",
"effectionx", "state", "reducer", and "middleware" so the final "keywords" array
in package.json contains only allowed keywords (e.g., ["reactivity"] or another
approved set appropriate for this package).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 111d2dc7-d126-4a61-80ad-60be06da6a7c
📒 Files selected for processing (28)
AGENTS.mdbdd/package.jsonchain/package.jsoncontext-api/package.jsonconverge/package.jsoneffect-ts/package.jsonfetch/package.jsonfs/package.jsonfx/package.jsoninline/package.jsonjsonl-store/package.jsonmiddleware/package.jsonnode/package.jsonprocess/package.jsonraf/package.jsonscope-eval/package.jsonsignals/package.jsonstate-api/package.jsonstream-helpers/package.jsonstream-yaml/package.jsontask-buffer/package.jsontest-adapter/package.jsontimebox/package.jsontinyexec/package.jsonvitest/package.jsonwatch/package.jsonwebsocket/package.jsonworker/package.json
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Pre-existing gap caught by Package.json Metadata policy on PR #216: inline/package.json was missing the required keywords field. Add ["concurrency"] (an approved category that fits the package's purpose of optimizing concurrent operation execution).
cowboyd
left a comment
There was a problem hiding this comment.
Sometimes it is nice to have the sources for debugging, but I get the desire to have smaller packages. What's the reason?
Drop source .ts files from published tarballs by adding (or narrowing) a top-level `files` array on every workspace-published package. The top-level `exports` map keeps the `development` condition so in-repo workspace resolution continues to read source directly via pnpm symlinks; .internal/strip-dev-exports.ts already removes that condition at publish time. `publishConfig.exports` / `publishConfig.files` were considered and rejected — npm (verified on 11.6.2) does not use them to override top-level `exports` or `files`. The top-level `files` field is the only mechanism that actually shapes the tarball. Per-package shape: - bdd: ["dist", "mod.deno.ts"] — ./deno subpath serves source directly - inline: ["dist", "swc/.../swc_plugin_inline.wasm"] — WASM artifact - all other 25: ["dist"] Scope: 27 workspace-published @effectionx/* packages listed in pnpm-workspace.yaml. deno-deploy is deliberately excluded (commented out of the workspace as deprecated). Verified by rsyncing the edited tree into a sim directory, running `node --experimental-strip-types .internal/strip-dev-exports.ts`, then extracting real `npm pack` tarballs for fx, context-api, bdd, and node. All tarballs contain no `development` export condition and no source .ts except bdd/mod.deno.ts (intentional). AGENTS.md updated so the documented `files` requirement matches the new shape.
Pre-existing gap caught by Package.json Metadata policy on PR #216: inline/package.json was missing the required keywords field. Add ["concurrency"] (an approved category that fits the package's purpose of optimizing concurrent operation execution).
95e9d99 to
6469179CompareUh oh!
There was an error while loading. Please reload this page.
Summary
filesarray on all 27 workspace-published@effectionx/*packages so source.tsno longer ships in the tarball.exportsis untouched —.internal/strip-dev-exports.tsalready removes thedevelopmentcondition at publish time, and workspace dev keeps resolving source via pnpm symlinks + thedevelopmentcondition.AGENTS.mdupdated so the documentedfilesrequirement matches the new shape.Why not
publishConfigpublishConfig.exports/publishConfig.fileswere tried first and rejected — verified on npm 11.6.2 that they do not override the top-levelexportsorfilesduringnpm pack. The top-levelfilesfield is the only mechanism that actually shapes the tarball.Per-package shape
bdd→["dist", "mod.deno.ts"]— the./denosubpath intentionally serves source.ts(Deno consumes.tsdirectly).inline→["dist", "swc/target/wasm32-wasip1/release/swc_plugin_inline.wasm"]— keeps the SWC WASM artifact.["dist"].Scope
27 workspace-published
@effectionx/*packages (source of truth:pnpm-workspace.yaml).deno-deployis deliberately excluded — it's commented out of the workspace as deprecated, so.internal/strip-dev-exports.tsdoesn't walk it andpnpm publishdoesn't publish it.Test plan
pnpm fmt:check— cleanpnpm check— cleanpnpm test— 104 real package test files pass (767 tests, 12 pre-existing skips). The 50 "failures" are all inside.claude/worktrees/agent-*— stale git worktrees that Vitest picks up as duplicate test files; unrelated to this PR.rsyncedited tree →node --experimental-strip-types .internal/strip-dev-exports.ts→npm packonfx,context-api,bdd,node. Verified per tarball:"development"key anywhere in publishedexports.filesmatches the plan..tsin any tarball exceptbdd/mod.deno.ts(intentional).dist/present everywhere.inlinepost-strip manifest inspected directly:exportsclean,fileskeeps onlydist+ WASM. (WASM build itself requires a Rust toolchain upgrade unrelated to this PR.)Manifest-only change — per
.policies/version-bump.md, no changeset / version bump required.Summary by CodeRabbit
Documentation
Chores
concurrencykeyword to improve package discoverability.