Repository navigation
chore: formalize & guard ESM-only (no CJS, ESM-native) — closes #20 - #30
Conversation
- add @arethetypeswrong/cli (esm-only profile) to check:publish alongside publint - drop legacy top-level main/types from core & vite (exports-only, uniform) - add sideEffects:false across all packages - document ESM-only (Node 18+, require unsupported) in each README - one-line ESM-only policy comment in every tsup.config.ts - rename scripts/verify-svelte-import.mjs -> .js (.js is ESM under type:module) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 51 minutes and 50 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. 📝 WalkthroughWalkthroughThis PR formalizes the ESM-only stance across all packages by removing legacy top-level ChangesESM-only Guard
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Pull request overview
This PR formalizes the repo’s ESM-only posture by tightening package entrypoints and adding a CI guard that checks ESM/type resolution, while also documenting the ESM-only requirement across packages.
Changes:
- Add
@arethetypeswrong/cli(attw) and wire it intocheck:publishvia a newcheck:typesscript to guard ESM-only type resolution in CI. - Standardize packages on
exports-only by removing legacy top-levelmain/typesfrom@svelte-vitals/coreand@svelte-vitals/vite, and addsideEffects: falseconsistently. - Update READMEs and tsup configs to explicitly state “ESM-only (Node 18+)” and record the “never add cjs” policy.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| scripts/verify-svelte-import.js | Updates runtime command docs and clarifies .js is ESM under the repo’s type: module. |
| pnpm-workspace.yaml | Adds @arethetypeswrong/cli to the workspace catalog. |
| pnpm-lock.yaml | Locks @arethetypeswrong/cli and its transitive deps. |
| packages/vite/tsup.config.ts | Adds an explicit “ESM-only; never add cjs” policy comment. |
| packages/vite/README.md | Documents ESM-only / Node 18+ / require() unsupported. |
| packages/vite/package.json | Adds sideEffects: false and removes legacy main/types (exports-only). |
| packages/mcp/README.md | Updates ESM-only wording to the new standardized phrasing. |
| packages/mcp/package.json | Adds sideEffects: false. |
| packages/core/tsup.config.ts | Adds an explicit “ESM-only; never add cjs” policy comment. |
| packages/core/README.md | Documents ESM-only / Node 18+ / require() unsupported. |
| packages/core/package.json | Removes legacy main/types (exports-only). |
| packages/cli/tsup.config.ts | Adds an explicit “ESM-only; never add cjs” policy comment. |
| packages/cli/README.md | Documents ESM-only / Node 18+ / require() unsupported. |
| packages/cli/package.json | Adds sideEffects: false. |
| package.json | Splits check:publish into publint + new attw-based check:types and adds attw devDependency. |
| .changeset/esm-only-guard.md | Adds a changeset describing the ESM-only formalization and CI guard. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Dropping top-level main/types can affect consumers resolving entry points without exports support, so a patch (which implies compatibility) is too weak for those two packages; cli/mcp only gain additive sideEffects and stay patch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Make the documented "Node 18+" runtime floor machine-enforceable so npm install warnings, attw, and publint align with the README claim. Additive declaration — core/vite stay minor (entry-point change), cli/mcp stay patch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/README.md`:
- Around line 13-14: The README.md file has a Markdown formatting violation
where line 14 is a blank line within a blockquote block that lacks the required
blockquote marker. To fix this, add the `> ` prefix to the blank line (line 14)
that sits between the blockquote starting at line 13 and the content continuing
at line 15 to properly maintain blockquote formatting according to Markdown
specifications.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 59cc95ff-3fe9-4a9b-9aaa-1f096f1692e6
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
.changeset/esm-only-guard.mdpackage.jsonpackages/cli/README.mdpackages/cli/package.jsonpackages/cli/tsup.config.tspackages/core/README.mdpackages/core/package.jsonpackages/core/tsup.config.tspackages/mcp/README.mdpackages/mcp/package.jsonpackages/vite/README.mdpackages/vite/package.jsonpackages/vite/tsup.config.tspnpm-workspace.yamlscripts/verify-svelte-import.js
Two adjacent blockquotes (ESM-only note + [\!NOTE] alert) separated by a blank line tripped markdownlint MD028. Merging them with `>` would have broken the GitHub alert (its first line must be `[\!NOTE]`), so move the ESM-only line into the top tagline blockquote instead — alert stays intact. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
Locks in the project's ESM-only stance and guards it in CI so CJS can't creep back. Closes #20.
Changes
@arethetypeswrong/cli(attw) to the publish check. Newcheck:typesruns attw with--profile esm-onlyover all four packages;check:publishnow runspublint+ attw. The esm-only profile intentionally ignores thenode10/require-from-CJS cases (expected for ESM-only packages) and confirmsnode16 (ESM)+bundlerresolve cleanly.main/typesfrom@svelte-vitals/coreand@svelte-vitals/vite; every package is nowexports-only (matching the CLI and MCP packages).sideEffects: falsetosvelte-vitals,@svelte-vitals/vite, and@svelte-vitals/mcp(core already had it).require()unsupported by design.ESM-only by design (#20) — never add 'cjs'.comment in everytsup.config.ts..mjsconsistency — renamescripts/verify-svelte-import.mjs→.js(a plain.jsis already ESM under the repo'stype: module).Non-goals
No CJS or dual (ESM+CJS) build — the whole point is to stay ESM-only.
Validation
pnpm -r test— 198 passed (core 70, vite 31, cli 88, mcp 9)pnpm -r typecheck,pnpm build,pnpm lint,pnpm check:publint— greenattw --pack . --profile esm-only— clean for all four packages (node16 ESM+bundler🟢)🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
Chores
Documentation