-
-
Notifications
You must be signed in to change notification settings - Fork 611
chore(repo): add ESM-only upgrade playbook #1934
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
60d4d79
e0094fc
7038aeb
63e1859
d877991
ca2ba63
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,149 @@ | ||
| # Upgrade a plugin package to ESM-only (packages/<name>) | ||
|
|
||
| Upgrade a single plugin under `packages/<name>` to publish ESM-only output with TypeScript-emitted JS and declarations. | ||
|
|
||
| ## Prerequisites | ||
|
|
||
| - Repo already contains shared config at `.config/tsconfig.base.json` and `.config/tsconfig.plugin.json` and (optionally) `.config/vitest.config.mts` from prior migrations. | ||
| - Scope constraint: make changes only inside the target package directory (e.g., `packages/alias`). Do not add or edit files outside `packages/<name>`. | ||
| - Local Node 20.19+ to run builds and tests. | ||
|
|
||
| ## Steps | ||
|
|
||
| 1. Identify the target package | ||
|
|
||
| - Set a shell variable for reuse: `PKG=packages/<name>`. | ||
|
|
||
| 2. Package metadata: ESM-only and minimums | ||
|
|
||
| - Edit `$PKG/package.json`: | ||
|
|
||
| - Set `"type": "module"`. | ||
| - Replace legacy `main/module/exports.require` with an ESM-only export mapped via the explicit `"."` entry for broad tooling compatibility: | ||
| ```json | ||
| { | ||
| "exports": { | ||
| ".": { | ||
| "types": "./dist/index.d.ts", | ||
| "import": "./dist/index.js", | ||
| "default": "./dist/index.js" | ||
| } | ||
| }, | ||
| "types": "./dist/index.d.ts" | ||
| } | ||
| ``` | ||
| - Set minimums: `"engines": { "node": ">=20.19.0" }` and `"peerDependencies": { "rollup": ">=4.0.0" }`. | ||
| - Keep `rollup` as a devDependency only if tests use it. Otherwise remove it. | ||
| - Ensure published files include build output and standard docs: | ||
| ```json | ||
| "files": ["dist", "README.md", "LICENSE"] | ||
| ``` | ||
| Notes: | ||
| - `package.json` `files` does not support negation patterns. If an existing `package.json` contains "files": [ ..., "!dist/**/*.map", ... ], remove the negated entry—negation is not supported and will be ignored. | ||
| - Always publish source maps. Do not exclude `dist/**/*.map` via `.npmignore`, and do not disable `sourceMap`/`declarationMap` for published packages. If a package currently excludes maps (via `.npmignore` or tsconfig), remove those exclusions so maps are included. | ||
|
|
||
| 3. Build scripts: TypeScript emit to dist | ||
|
|
||
| - Prefer a tsc-only build for packages that do not need bundling: | ||
| - In `$PKG/package.json`, set scripts: | ||
| ```json | ||
| "prebuild": "del-cli dist", | ||
| "build": "tsc --project tsconfig.json", | ||
| "pretest": "pnpm build", | ||
| "prerelease": "pnpm build", | ||
| "prepare": "if [ ! -d 'dist' ]; then pnpm build; fi" | ||
| ``` | ||
shellscape marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| - If this package still needs bundling for tests/examples, keep its Rollup config but point inputs at the TypeScript output in `dist/` instead of sources. | ||
|
|
||
| 4. TypeScript config: use the shared plugin config (symlink) | ||
|
|
||
| - Replace any existing `$PKG/tsconfig.json` with a symlink to the shared plugin config (`.config/tsconfig.plugin.json`), which already enables emit to `dist/` and declaration maps: | ||
| ```bash | ||
| # from repo root | ||
| ln -snf ../../.config/tsconfig.plugin.json "$PKG/tsconfig.json" | ||
| git add "$PKG/tsconfig.json" | ||
| ``` | ||
| On Windows PowerShell, you can run: | ||
| ```powershell | ||
| # from repo root | ||
| $pkg = 'packages/<name>' | ||
| New-Item -ItemType SymbolicLink -Path "$pkg/tsconfig.json" -Target (Resolve-Path ".config/tsconfig.plugin.json") -Force | ||
| git add "$pkg/tsconfig.json" | ||
| ``` | ||
| The shared config content lives at `.config/tsconfig.plugin.json`. | ||
| - Delete any package-local `rollup` build scripts that produced CJS, and remove any `types/` folder if declarations were hand-authored (they will now be generated). | ||
|
|
||
| 5. Source: convert to pure ESM and modern Node APIs | ||
|
|
||
| - Replace `require`, `module.exports`, and `__dirname` patterns with ESM equivalents. | ||
| - Use `node:` specifiers for built-ins (e.g., `import path from 'node:path'`). | ||
| - Prefer the latest ES APIs: use `import.meta.dirname` and `import.meta.filename` (Node ≥20.11) instead of re‑creating them via `fileURLToPath`. | ||
|
|
||
| ```ts | ||
| import path from 'node:path'; | ||
|
|
||
| const here = import.meta.dirname; | ||
| // const file = import.meta.filename; | ||
| const pkgJson = path.join(here, 'package.json'); | ||
| ``` | ||
|
Comment on lines
+83
to
+88
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using a default import for Node built-ins can be confusing in TS projects without SuggestionPrefer a namespace import in the example for broad compatibility: import * as path from 'node:path';
const here = import.meta.dirname;
// const file = import.meta.filename;
const pkgJson = path.join(here, 'package.json');Reply with "@CharlieHelps yes please" if you'd like me to update the snippet accordingly. |
||
|
|
||
| Use URL utilities only when specifically needed (e.g., for non‑file module URLs): `fileURLToPath(new URL('.', import.meta.url))`. | ||
|
|
||
| - Inline and export public types from `src/index.ts`; avoid separate `types/` unless unavoidable. | ||
| - Conversion rules for file types: | ||
| - Always convert any `.js` in `src/` to `.ts`. | ||
| - Never convert files in test `fixtures/` to TypeScript—keep fixtures exactly as authored. | ||
| - Always convert any `.js` in `test/` to `.ts` (test sources only; exclude `test/**/fixtures/**`). | ||
|
|
||
| 6. Tests: order of operations (AVA → Vitest) and ESM | ||
|
|
||
| - Remove CJS-specific branches/assertions from tests. | ||
| - Follow this sequence for reliability: | ||
| 1. After JS→TS conversion in `src/` and `test/` and after any `package.json` changes, always run the AVA test suite. | ||
| 2. Only after verifying AVA tests run without modifications, convert the AVA tests to Vitest. | ||
| 3. After converting tests to Vitest, always run the Vitest suite after any subsequent `src/` change. | ||
| - Note: If a package already uses Vitest, start at step 3 and skip AVA‑specific steps. | ||
| - Ensure Rollup bundles created in tests are `await bundle.close()`-d to avoid leaks. | ||
|
|
||
| 7. Clean up package artifacts | ||
| - Remove obsolete files that are no longer used by ESM-only publishing (examples): | ||
| - `$PKG/rollup.config.*` if switching to tsc-only. | ||
| - `$PKG/types/**` once declarations are generated to `dist/`. | ||
|
|
||
| ## Verify | ||
|
|
||
| - Build succeeds and emits JS and d.ts to `dist/`: | ||
| ```bash | ||
| pnpm -C $PKG build | ||
| tree $PKG/dist | sed -n '1,80p' | ||
| ``` | ||
|
Comment on lines
+116
to
+119
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
SuggestionSuggest portable alternatives:
find "$PKG/dist" -maxdepth 2 -type f | sed -n '1,80p'
Get-ChildItem -Recurse -Depth 2 "$pkg/dist" | Select-Object -First 80Reply with "@CharlieHelps yes please" if you’d like me to update the Verify section with these alternatives. |
||
| - Symlink exists and points at the shared config: | ||
| ```bash | ||
| test -L "$PKG/tsconfig.json" && ls -l "$PKG/tsconfig.json" || (echo "tsconfig.json symlink missing" && exit 1) | ||
| ``` | ||
| - Type declarations resolve for consumers: | ||
| ```bash | ||
| jq -r '.types, .exports["."].types, .exports["."].import' $PKG/package.json | ||
| ``` | ||
|
Comment on lines
+124
to
+127
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The verify step checks SuggestionExpand the check and add a Node-based alternative:
jq -r '.types, .exports["."].types, .exports["."].import, .exports["."].default' "$PKG/package.json"
node -e "const fs=require('node:fs');const pkg=JSON.parse(fs.readFileSync(process.argv[1],'utf8'));console.log(pkg.types,pkg.exports?.['.']?.types,pkg.exports?.['.']?.import,pkg.exports?.['.']?.default)" "$PKG/package.json"Reply with "@CharlieHelps yes please" if you want me to apply these updates. |
||
| - Runtime smoke (Node ESM import works): | ||
| ```bash | ||
| node -e "import('file://$PWD/$PKG/dist/index.js').then(() => console.log('ok'))" | ||
| ``` | ||
|
Comment on lines
+128
to
+131
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The runtime smoke test uses SuggestionReplace the command with a cross‑platform Node one‑liner that resolves the file path safely: node -e "const {resolve}=require('node:path');const {pathToFileURL}=require('node:url');import(pathToFileURL(resolve(process.argv[1]))).then(()=>console.log('ok'))" "$PKG/dist/index.js"Reply with "@CharlieHelps yes please" and I’ll update the Verify snippet accordingly. |
||
| - Tests pass for the package (runner may be AVA or Vitest depending on the package): | ||
| ```bash | ||
| pnpm -C $PKG test | ||
| ``` | ||
|
|
||
| ## Rollback | ||
|
|
||
| - Revert the package directory to the previous commit (modern Git): | ||
| ```bash | ||
| git restore -SW $PKG | ||
| ``` | ||
| - If needed, `git reset --hard HEAD~1` when this package’s change is isolated on a feature branch. | ||
|
|
||
| ## References | ||
|
|
||
| - Alias migration (ESM-only) — PR #1926: feat(alias)!: ESM only. Update Node and Rollup minimum versions | ||
| - Task spec used for alias — Issue #1925 | ||
| - Shared TS configs used by packages — `.config/tsconfig.base.json`, `.config/tsconfig.plugin.json` | ||
Uh oh!
There was an error while loading. Please reload this page.