|
| 1 | +# ESM Migration Plan: Add File Extensions to Imports |
| 2 | + |
| 3 | +## Overview |
| 4 | + |
| 5 | +This document outlines the plan to migrate from TypeScript's deprecated `"moduleResolution": "node"` (node10) to `"moduleResolution": "node16"` or `"nodenext"`. This change is necessary because the published ESM packages have extensionless imports that don't work correctly in modern ESM environments. |
| 6 | + |
| 7 | +## Issues Fixed |
| 8 | + |
| 9 | +This migration will resolve the following issues: |
| 10 | + |
| 11 | +- **#154** - Upgrade `moduleResolution` from `node` to `node16` or `nodenext` in tsconfig |
| 12 | +- **#110** - Published ESM code has imports without file extensions |
| 13 | +- **#64** - expressions: ERR_MODULE_NOT_FOUND attempting to run example demo script |
| 14 | +- **#146** - Can not import `@actions/workflow-parser` |
| 15 | + |
| 16 | +## Problem Statement |
| 17 | + |
| 18 | +### Current State |
| 19 | + |
| 20 | +All packages use `"moduleResolution": "node"`: |
| 21 | + |
| 22 | +| Package | moduleResolution | TypeScript | |
| 23 | +|---------|------------------|------------| |
| 24 | +| expressions | `"node"` | ^4.7.4 | |
| 25 | +| workflow-parser | `"node"` | ^4.8.4 | |
| 26 | +| languageservice | `"node"` | ^4.8.4 | |
| 27 | +| languageserver | `"node"` | ^4.8.4 | |
| 28 | +| browser-playground | `"Node16"` ✅ | ^4.9.4 | |
| 29 | + |
| 30 | +This causes TypeScript to emit code like: |
| 31 | +```javascript |
| 32 | +// Published to npm - INVALID ESM |
| 33 | +export { Expr } from "./ast"; // Missing .js extension! |
| 34 | +``` |
| 35 | + |
| 36 | +### Why This Fails |
| 37 | + |
| 38 | +ESM in Node.js 12+ **requires** explicit file extensions. When users try to import these packages: |
| 39 | + |
| 40 | +```javascript |
| 41 | +// User's code |
| 42 | +import { Expr } from "@actions/expressions"; |
| 43 | +``` |
| 44 | + |
| 45 | +Node.js fails with: |
| 46 | +``` |
| 47 | +Error [ERR_MODULE_NOT_FOUND]: Cannot find module '.../node_modules/@actions/expressions/dist/ast' |
| 48 | +``` |
| 49 | + |
| 50 | +## Migration Strategy |
| 51 | + |
| 52 | +### Option A: TypeScript 5.7+ with `rewriteRelativeImportExtensions` (Recommended) |
| 53 | + |
| 54 | +TypeScript 5.7 introduced a new compiler option that automatically rewrites `.ts` extensions to `.js` in output: |
| 55 | + |
| 56 | +```jsonc |
| 57 | +{ |
| 58 | + "compilerOptions": { |
| 59 | + "moduleResolution": "node16", // or "nodenext" |
| 60 | + "rewriteRelativeImportExtensions": true |
| 61 | + } |
| 62 | +} |
| 63 | +``` |
| 64 | + |
| 65 | +**Source code:** |
| 66 | +```typescript |
| 67 | +import { Expr } from "./ast.ts"; |
| 68 | +``` |
| 69 | + |
| 70 | +**Compiled output:** |
| 71 | +```javascript |
| 72 | +export { Expr } from "./ast.js"; |
| 73 | +``` |
| 74 | + |
| 75 | +**Pros:** |
| 76 | +- Source uses `.ts` extensions (matches actual files) |
| 77 | +- Works with Deno (which requires `.ts` extensions) |
| 78 | +- TypeScript automatically transforms to `.js` |
| 79 | +- Modern, forward-looking approach |
| 80 | + |
| 81 | +**Cons:** |
| 82 | +- Requires TypeScript 5.7+ |
| 83 | +- Relatively new feature |
| 84 | + |
| 85 | +### Option B: Manual `.js` Extensions |
| 86 | + |
| 87 | +Use `.js` extensions in source TypeScript files: |
| 88 | + |
| 89 | +```typescript |
| 90 | +import { Expr } from "./ast.js"; // Points to .ts file, but use .js extension |
| 91 | +``` |
| 92 | + |
| 93 | +**Pros:** |
| 94 | +- Works with TypeScript 4.7+ (with node16 moduleResolution) |
| 95 | +- Well-established pattern |
| 96 | + |
| 97 | +**Cons:** |
| 98 | +- Confusing - `.js` files don't exist at write time |
| 99 | +- Doesn't work with Deno out of the box |
| 100 | + |
| 101 | +### Recommendation |
| 102 | + |
| 103 | +**Use Option A** - TypeScript 5.7+ with `rewriteRelativeImportExtensions`: |
| 104 | +- Cleaner developer experience (`.ts` imports match actual files) |
| 105 | +- Better Deno compatibility |
| 106 | +- TypeScript 5.x upgrade is already on the roadmap (see Dependabot PRs #208-212) |
| 107 | + |
| 108 | +## Scope of Changes |
| 109 | + |
| 110 | +### Statistics |
| 111 | + |
| 112 | +- **~73 source files** need import updates |
| 113 | +- **~176 relative imports** need `.ts` extensions added |
| 114 | +- **5 packages** need tsconfig updates (browser-playground already uses node16) |
| 115 | +- **6 JSON imports** need attention |
| 116 | + |
| 117 | +### Package-by-Package Breakdown |
| 118 | + |
| 119 | +#### expressions/ |
| 120 | +- tsconfig.json: Update `moduleResolution` |
| 121 | +- Add `.ts` extensions to all relative imports |
| 122 | +- Files: ~15 source files |
| 123 | + |
| 124 | +#### workflow-parser/ |
| 125 | +- tsconfig.json: Update `moduleResolution` |
| 126 | +- Add `.ts` extensions to all relative imports |
| 127 | +- JSON import: `workflow-v1.0.min.json` - needs `with { type: "json" }` or type assertion |
| 128 | +- Files: ~25 source files |
| 129 | + |
| 130 | +#### languageservice/ |
| 131 | +- tsconfig.json: Update `moduleResolution` |
| 132 | +- Add `.ts` extensions to all relative imports |
| 133 | +- JSON imports: webhooks, schedule, workflow_call, descriptions - need handling |
| 134 | +- Files: ~30 source files |
| 135 | + |
| 136 | +#### languageserver/ |
| 137 | +- tsconfig.json: Update `moduleResolution` |
| 138 | +- Add `.ts` extensions to all relative imports |
| 139 | +- Files: ~10 source files |
| 140 | + |
| 141 | +#### browser-playground/ |
| 142 | +- Already uses `"moduleResolution": "Node16"` ✅ |
| 143 | +- May need import extension updates |
| 144 | +- Bundled via webpack, so may have different requirements |
| 145 | + |
| 146 | +### JSON Import Handling |
| 147 | + |
| 148 | +JSON imports require special handling in node16/nodenext. Options: |
| 149 | + |
| 150 | +1. **Import Attributes (Node 20.10+)** |
| 151 | + ```typescript |
| 152 | + import schema from "./workflow-v1.0.json" with { type: "json" }; |
| 153 | + ``` |
| 154 | + Requires: TypeScript 5.3+, Node 20.10+ |
| 155 | + |
| 156 | +2. **fs.readFileSync at runtime** |
| 157 | + ```typescript |
| 158 | + const schema = JSON.parse(fs.readFileSync(new URL("./schema.json", import.meta.url), "utf8")); |
| 159 | + ``` |
| 160 | + Works with any Node version but requires runtime file access |
| 161 | + |
| 162 | +3. **Keep resolveJsonModule with assertion** |
| 163 | + ```typescript |
| 164 | + import schema from "./schema.json" assert { type: "json" }; |
| 165 | + ``` |
| 166 | + Note: `assert` is deprecated in favor of `with` |
| 167 | + |
| 168 | +**Recommendation:** Use Import Attributes (`with { type: "json" }`) since we already require Node >= 18 and will bump TypeScript to 5.x. |
| 169 | + |
| 170 | +## Implementation Steps |
| 171 | + |
| 172 | +### Phase 1: Prerequisites (Wait for merge) |
| 173 | + |
| 174 | +- [ ] Merge all pending PRs to avoid conflicts |
| 175 | + - PR #242 (activity types) |
| 176 | + - Any other pending PRs |
| 177 | + |
| 178 | +### Phase 2: TypeScript Upgrade |
| 179 | + |
| 180 | +- [ ] Upgrade TypeScript to 5.7+ in all packages |
| 181 | +- [ ] Merge Dependabot PRs #208-212 or create unified upgrade PR |
| 182 | +- [ ] Verify all tests pass after upgrade |
| 183 | + |
| 184 | +### Phase 3: tsconfig Updates |
| 185 | + |
| 186 | +Update each package's `tsconfig.json`: |
| 187 | + |
| 188 | +```jsonc |
| 189 | +{ |
| 190 | + "compilerOptions": { |
| 191 | + "module": "node16", // or "nodenext" |
| 192 | + "moduleResolution": "node16", // or "nodenext" |
| 193 | + "rewriteRelativeImportExtensions": true |
| 194 | + } |
| 195 | +} |
| 196 | +``` |
| 197 | + |
| 198 | +### Phase 4: Add Extensions to Imports |
| 199 | + |
| 200 | +For each package, update all relative imports: |
| 201 | + |
| 202 | +```typescript |
| 203 | +// Before |
| 204 | +import { Expr } from "./ast"; |
| 205 | +import { Parser } from "./parser"; |
| 206 | + |
| 207 | +// After |
| 208 | +import { Expr } from "./ast.ts"; |
| 209 | +import { Parser } from "./parser.ts"; |
| 210 | +``` |
| 211 | + |
| 212 | +**Automation approach:** |
| 213 | +```bash |
| 214 | +# Can use sed/perl or a codemod tool |
| 215 | +find src -name "*.ts" -exec sed -i "s/from '\.\//from '.\\//g" {} \; |
| 216 | +# More sophisticated regex needed - consider using ts-morph or jscodeshift |
| 217 | +``` |
| 218 | + |
| 219 | +### Phase 5: JSON Import Updates |
| 220 | + |
| 221 | +Update JSON imports to use import attributes: |
| 222 | + |
| 223 | +```typescript |
| 224 | +// Before |
| 225 | +import schema from "./workflow-v1.0.min.json"; |
| 226 | + |
| 227 | +// After |
| 228 | +import schema from "./workflow-v1.0.min.json" with { type: "json" }; |
| 229 | +``` |
| 230 | + |
| 231 | +### Phase 6: Verification |
| 232 | + |
| 233 | +- [ ] Run `npm run build` in all packages |
| 234 | +- [ ] Run `npm test` in all packages |
| 235 | +- [ ] Test importing published packages in: |
| 236 | + - [ ] Node.js ESM mode (`"type": "module"`) |
| 237 | + - [ ] Vite project |
| 238 | + - [ ] Deno (bonus) |
| 239 | +- [ ] Verify browser-playground still works |
| 240 | + |
| 241 | +### Phase 7: Documentation |
| 242 | + |
| 243 | +- [ ] Update READMEs with any new requirements |
| 244 | +- [ ] Add migration notes to CHANGELOG |
| 245 | + |
| 246 | +## Risks and Mitigations |
| 247 | + |
| 248 | +| Risk | Impact | Mitigation | |
| 249 | +|------|--------|------------| |
| 250 | +| Breaking change for consumers | High | Semver major version bump | |
| 251 | +| Import attributes not supported in older bundlers | Medium | Test with webpack, vite, rollup | |
| 252 | +| Some edge cases with re-exports | Low | Careful testing | |
| 253 | +| browser-playground uses webpack | Low | Webpack handles bundling differently | |
| 254 | + |
| 255 | +## Timeline |
| 256 | + |
| 257 | +1. **Wait for pending PRs** - ~1 week |
| 258 | +2. **TypeScript upgrade** - 1 day |
| 259 | +3. **Import migration** - 2-3 days |
| 260 | +4. **Testing & verification** - 1 day |
| 261 | +5. **Documentation** - 0.5 day |
| 262 | + |
| 263 | +**Total: ~1-2 weeks** after dependencies are ready |
| 264 | + |
| 265 | +## References |
| 266 | + |
| 267 | +- [TypeScript moduleResolution reference](https://www.typescriptlang.org/docs/handbook/modules/reference.html) |
| 268 | +- [TypeScript 5.7 rewriteRelativeImportExtensions](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-5-7.html#path-rewriting-for-relative-paths) |
| 269 | +- [Node.js ESM mandatory extensions](https://nodejs.org/api/esm.html#mandatory-file-extensions) |
| 270 | +- [Import Attributes proposal](https://github.com/tc39/proposal-import-attributes) |
| 271 | +- [Community fork that works](https://github.com/boxbuild-io/actions-languageservices/commit/077fb2b58dfd2cca3d6e3df1fdf9e26e75db24ae) |
0 commit comments