Update ESM migration plan with findings and workarounds
This commit is contained in:
+250
-133
@@ -81,6 +81,7 @@ export { Expr } from "./ast.js";
|
||||
**Cons:**
|
||||
- Requires TypeScript 5.7+
|
||||
- Relatively new feature
|
||||
- **BUG:** See "Known Issues" section below
|
||||
|
||||
### Option B: Manual `.js` Extensions
|
||||
|
||||
@@ -93,6 +94,7 @@ import { Expr } from "./ast.js"; // Points to .ts file, but use .js extension
|
||||
**Pros:**
|
||||
- Works with TypeScript 4.7+ (with node16 moduleResolution)
|
||||
- Well-established pattern
|
||||
- No post-processing needed
|
||||
|
||||
**Cons:**
|
||||
- Confusing - `.js` files don't exist at write time
|
||||
@@ -100,172 +102,287 @@ import { Expr } from "./ast.js"; // Points to .ts file, but use .js extension
|
||||
|
||||
### Recommendation
|
||||
|
||||
**Use Option A** - TypeScript 5.7+ with `rewriteRelativeImportExtensions`:
|
||||
- Cleaner developer experience (`.ts` imports match actual files)
|
||||
- Better Deno compatibility
|
||||
- TypeScript 5.x upgrade is already on the roadmap (see Dependabot PRs #208-212)
|
||||
**Use Option A** with workarounds for known issues (see below).
|
||||
|
||||
## Scope of Changes
|
||||
---
|
||||
|
||||
### Statistics
|
||||
## Known Issues and Workarounds (December 2025)
|
||||
|
||||
- **~73 source files** need import updates
|
||||
- **~176 relative imports** need `.ts` extensions added
|
||||
- **5 packages** need tsconfig updates (browser-playground already uses node16)
|
||||
- **6 JSON imports** need attention
|
||||
### 1. TypeScript Version Conflicts in Monorepo
|
||||
|
||||
### Package-by-Package Breakdown
|
||||
**Problem:** The root `node_modules/typescript` was version 4.9.5 (pulled in by `ts-node` and `tsutils` dependencies), while workspace packages specified `^5.8.3`.
|
||||
|
||||
#### expressions/
|
||||
- tsconfig.json: Update `moduleResolution`
|
||||
- Add `.ts` extensions to all relative imports
|
||||
- Files: ~15 source files
|
||||
**Symptoms:**
|
||||
- `npx tsc --version` showed 4.9.5
|
||||
- `require('typescript').version` in ts-jest showed 5.8.3
|
||||
- Confusing build failures
|
||||
|
||||
#### workflow-parser/
|
||||
- tsconfig.json: Update `moduleResolution`
|
||||
- Add `.ts` extensions to all relative imports
|
||||
- JSON import: `workflow-v1.0.min.json` - needs `with { type: "json" }` or type assertion
|
||||
- Files: ~25 source files
|
||||
|
||||
#### languageservice/
|
||||
- tsconfig.json: Update `moduleResolution`
|
||||
- Add `.ts` extensions to all relative imports
|
||||
- JSON imports: webhooks, schedule, workflow_call, descriptions - need handling
|
||||
- Files: ~30 source files
|
||||
|
||||
#### languageserver/
|
||||
- tsconfig.json: Update `moduleResolution`
|
||||
- Add `.ts` extensions to all relative imports
|
||||
- Files: ~10 source files
|
||||
|
||||
#### browser-playground/
|
||||
- Already uses `"moduleResolution": "Node16"` ✅
|
||||
- May need import extension updates
|
||||
- Bundled via webpack, so may have different requirements
|
||||
|
||||
### JSON Import Handling
|
||||
|
||||
JSON imports require special handling in node16/nodenext. Options:
|
||||
|
||||
1. **Import Attributes (Node 20.10+)**
|
||||
```typescript
|
||||
import schema from "./workflow-v1.0.json" with { type: "json" };
|
||||
```
|
||||
Requires: TypeScript 5.3+, Node 20.10+
|
||||
|
||||
2. **fs.readFileSync at runtime**
|
||||
```typescript
|
||||
const schema = JSON.parse(fs.readFileSync(new URL("./schema.json", import.meta.url), "utf8"));
|
||||
```
|
||||
Works with any Node version but requires runtime file access
|
||||
|
||||
3. **Keep resolveJsonModule with assertion**
|
||||
```typescript
|
||||
import schema from "./schema.json" assert { type: "json" };
|
||||
```
|
||||
Note: `assert` is deprecated in favor of `with`
|
||||
|
||||
**Recommendation:** Use Import Attributes (`with { type: "json" }`) since we already require Node >= 18 and will bump TypeScript to 5.x.
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
### Phase 1: Prerequisites (Wait for merge)
|
||||
|
||||
- [ ] Merge all pending PRs to avoid conflicts
|
||||
- PR #242 (activity types)
|
||||
- Any other pending PRs
|
||||
|
||||
### Phase 2: TypeScript Upgrade
|
||||
|
||||
- [ ] Upgrade TypeScript to 5.7+ in all packages
|
||||
- [ ] Merge Dependabot PRs #208-212 or create unified upgrade PR
|
||||
- [ ] Verify all tests pass after upgrade
|
||||
|
||||
### Phase 3: tsconfig Updates
|
||||
|
||||
Update each package's `tsconfig.json`:
|
||||
|
||||
```jsonc
|
||||
**Solution:** Add npm overrides in root `package.json`:
|
||||
```json
|
||||
{
|
||||
"compilerOptions": {
|
||||
"module": "node16", // or "nodenext"
|
||||
"moduleResolution": "node16", // or "nodenext"
|
||||
"rewriteRelativeImportExtensions": true
|
||||
"overrides": {
|
||||
"typescript": "5.8.3"
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Phase 4: Add Extensions to Imports
|
||||
### 2. ts-jest Compatibility with TypeScript 5.9+
|
||||
|
||||
For each package, update all relative imports:
|
||||
**Problem:** ts-jest 29.4.6 uses `typescript.JSDocParsingMode.ParseAll` which doesn't exist in TypeScript's ES module exports.
|
||||
|
||||
```typescript
|
||||
// Before
|
||||
import { Expr } from "./ast";
|
||||
import { Parser } from "./parser";
|
||||
|
||||
// After
|
||||
import { Expr } from "./ast.ts";
|
||||
import { Parser } from "./parser.ts";
|
||||
**Error:**
|
||||
```
|
||||
TypeError: Cannot read properties of undefined (reading 'ParseAll')
|
||||
at Object.<anonymous> (node_modules/ts-jest/dist/compiler/ts-compiler.js:43:123)
|
||||
```
|
||||
|
||||
**Automation approach:**
|
||||
```bash
|
||||
# Can use sed/perl or a codemod tool
|
||||
find src -name "*.ts" -exec sed -i "s/from '\.\//from '.\\//g" {} \;
|
||||
# More sophisticated regex needed - consider using ts-morph or jscodeshift
|
||||
**Root Cause:** ts-jest accesses `typescript_1.default.JSDocParsingMode.ParseAll` but TypeScript has no default export in ESM.
|
||||
|
||||
**Solution:**
|
||||
- Use ts-jest 29.0.3 (older version that doesn't use this API)
|
||||
- OR wait for ts-jest fix
|
||||
- **Stay on TypeScript 5.8.3, not 5.9+**
|
||||
|
||||
### 3. TypeScript `rewriteRelativeImportExtensions` Bug with .d.ts Files
|
||||
|
||||
**Problem:** TypeScript's `rewriteRelativeImportExtensions: true` correctly rewrites `.ts` → `.js` in `.js` output files, but **incorrectly keeps `.ts` extensions in `.d.ts` declaration files**.
|
||||
|
||||
**Example:**
|
||||
- Source: `export { Expr } from "./ast.ts";`
|
||||
- Output `index.js`: `export { Expr } from "./ast.js";` ✅ Correct
|
||||
- Output `index.d.ts`: `export { Expr } from "./ast.ts";` ❌ Wrong (should be `.js`)
|
||||
|
||||
**Upstream Issue:** https://github.com/microsoft/TypeScript/issues/61037 (marked "Help Wanted", in Backlog, NOT FIXED as of Dec 2025)
|
||||
|
||||
**Workaround:** Post-process `.d.ts` files with a script. Create `script/fix-dts-extensions.cjs`:
|
||||
|
||||
```javascript
|
||||
#!/usr/bin/env node
|
||||
/**
|
||||
* Post-build script to fix TypeScript's rewriteRelativeImportExtensions bug
|
||||
* where .d.ts files get .ts extensions instead of .js extensions.
|
||||
* See: https://github.com/microsoft/TypeScript/issues/61037
|
||||
*/
|
||||
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
|
||||
function fixDtsFile(filePath) {
|
||||
let content = fs.readFileSync(filePath, 'utf8');
|
||||
const original = content;
|
||||
|
||||
// Replace .ts extensions in import/export statements with .js
|
||||
content = content.replace(/(from\s+["'])([^"']+)\.ts(["'])/g, '$1$2.js$3');
|
||||
content = content.replace(/(import\s*\(\s*["'])([^"']+)\.ts(["']\s*\))/g, '$1$2.js$3');
|
||||
content = content.replace(/(export\s+\*\s+from\s+["'])([^"']+)\.ts(["'])/g, '$1$2.js$3');
|
||||
|
||||
if (content !== original) {
|
||||
fs.writeFileSync(filePath, content, 'utf8');
|
||||
console.log(`Fixed: ${filePath}`);
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
function walkDir(dir, callback) {
|
||||
const files = fs.readdirSync(dir);
|
||||
for (const file of files) {
|
||||
const filePath = path.join(dir, file);
|
||||
const stat = fs.statSync(filePath);
|
||||
if (stat.isDirectory()) {
|
||||
walkDir(filePath, callback);
|
||||
} else if (file.endsWith('.d.ts')) {
|
||||
callback(filePath);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function main() {
|
||||
const args = process.argv.slice(2);
|
||||
if (args.length === 0) {
|
||||
console.error('Usage: fix-dts-extensions.cjs <dist-dir> [<dist-dir2> ...]');
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
let fixedCount = 0;
|
||||
for (const dir of args) {
|
||||
if (!fs.existsSync(dir)) {
|
||||
console.warn(`Directory not found: ${dir}`);
|
||||
continue;
|
||||
}
|
||||
walkDir(dir, (filePath) => {
|
||||
if (fixDtsFile(filePath)) {
|
||||
fixedCount++;
|
||||
}
|
||||
});
|
||||
}
|
||||
console.log(`Fixed ${fixedCount} .d.ts file(s)`);
|
||||
}
|
||||
|
||||
main();
|
||||
```
|
||||
|
||||
### Phase 5: JSON Import Updates
|
||||
**Note:** Use `.cjs` extension since root `package.json` has `"type": "module"`.
|
||||
|
||||
Update JSON imports to use import attributes:
|
||||
|
||||
```typescript
|
||||
// Before
|
||||
import schema from "./workflow-v1.0.min.json";
|
||||
|
||||
// After
|
||||
import schema from "./workflow-v1.0.min.json" with { type: "json" };
|
||||
**Usage in package.json build scripts:**
|
||||
```json
|
||||
{
|
||||
"scripts": {
|
||||
"build": "tsc --build tsconfig.build.json && node ../script/fix-dts-extensions.cjs dist"
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Phase 6: Verification
|
||||
### 4. languageserver Tests Hang
|
||||
|
||||
**Problem:** The languageserver tests hang indefinitely when running with the ESM configuration.
|
||||
|
||||
**Status:** Not fully diagnosed. Tests pass on main branch but hang on ESM branch.
|
||||
|
||||
**Possible causes:**
|
||||
- Jest ESM module resolution issues
|
||||
- Cross-package source mappings in jest.config.js
|
||||
- vscode-languageserver ESM compatibility issues
|
||||
- Specific test file causing hang (needs isolation testing)
|
||||
|
||||
**Investigation needed:**
|
||||
- Run individual test files to isolate the hanging test
|
||||
- Check if vscode-languageserver has ESM compatibility issues
|
||||
- Review jest configuration for problematic mappings
|
||||
- Try running with `--detectOpenHandles` flag
|
||||
|
||||
---
|
||||
|
||||
## Required Configuration Changes
|
||||
|
||||
### tsconfig.json (each package)
|
||||
|
||||
```json
|
||||
{
|
||||
"compilerOptions": {
|
||||
"module": "node16",
|
||||
"moduleResolution": "node16",
|
||||
"rewriteRelativeImportExtensions": true,
|
||||
"lib": ["ES2022"],
|
||||
"target": "ES2022"
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### jest.config.js (each package)
|
||||
|
||||
```javascript
|
||||
/** @type {import('ts-jest').JestConfigWithTsJest} */
|
||||
export default {
|
||||
preset: "ts-jest/presets/default-esm",
|
||||
moduleNameMapper: {
|
||||
"^(\\.{1,2}/.*)\\.js$": "$1",
|
||||
"^(\\.{1,2}/.*)\\.ts$": "$1",
|
||||
},
|
||||
transform: {
|
||||
"^.+\\.tsx?$": [
|
||||
"ts-jest",
|
||||
{
|
||||
useESM: true,
|
||||
isolatedModules: true,
|
||||
},
|
||||
],
|
||||
},
|
||||
moduleFileExtensions: ["ts", "js"],
|
||||
};
|
||||
```
|
||||
|
||||
### Root package.json
|
||||
|
||||
```json
|
||||
{
|
||||
"overrides": {
|
||||
"typescript": "5.8.3"
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Each workspace package.json
|
||||
|
||||
```json
|
||||
{
|
||||
"devDependencies": {
|
||||
"typescript": "^5.8.3",
|
||||
"ts-jest": "^29.0.3"
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Test Results (December 2025 Attempt)
|
||||
|
||||
| Package | Tests | Status |
|
||||
|---------|-------|--------|
|
||||
| expressions | 1068 | ✅ Pass |
|
||||
| workflow-parser | 292 | ✅ Pass |
|
||||
| languageservice | 452 | ✅ Pass |
|
||||
| languageserver | 6 files | ❌ Hangs (needs investigation) |
|
||||
|
||||
---
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
### Phase 1: Prerequisites
|
||||
- [ ] Merge all pending PRs to avoid conflicts
|
||||
- [ ] Ensure clean main branch state
|
||||
|
||||
### Phase 2: TypeScript & Dependencies
|
||||
- [ ] Add `"overrides": { "typescript": "5.8.3" }` to root package.json
|
||||
- [ ] Update all workspace packages to TypeScript ^5.8.3
|
||||
- [ ] Downgrade ts-jest to ^29.0.3 in all packages
|
||||
- [ ] Run `npm install` to apply changes
|
||||
|
||||
### Phase 3: tsconfig Updates
|
||||
- [ ] Update tsconfig.json in each package with node16 settings
|
||||
- [ ] Add `rewriteRelativeImportExtensions: true`
|
||||
|
||||
### Phase 4: Add .ts Extensions to Imports
|
||||
- [ ] Update all relative imports in source files to use `.ts` extensions
|
||||
- [ ] This was already done in PR #243
|
||||
|
||||
### Phase 5: Create Fix Script
|
||||
- [ ] Add `script/fix-dts-extensions.cjs` to repository
|
||||
- [ ] Update build scripts to run fix after tsc
|
||||
|
||||
### Phase 6: Fix languageserver Tests
|
||||
- [ ] Investigate why tests hang
|
||||
- [ ] Isolate problematic test file
|
||||
- [ ] Apply fix or workaround
|
||||
|
||||
### Phase 7: Verification
|
||||
- [ ] Run `npm run build` in all packages
|
||||
- [ ] Run `npm test` in all packages
|
||||
- [ ] Test importing published packages in:
|
||||
- [ ] Node.js ESM mode (`"type": "module"`)
|
||||
- [ ] Vite project
|
||||
- [ ] Deno (bonus)
|
||||
- [ ] Test importing published packages in Node.js ESM mode
|
||||
- [ ] Verify browser-playground still works
|
||||
|
||||
### Phase 7: Documentation
|
||||
### Phase 8: CI Updates
|
||||
- [ ] Update GitHub Actions workflows if needed
|
||||
- [ ] Ensure fix-dts-extensions.cjs runs in CI
|
||||
|
||||
- [ ] Update READMEs with any new requirements
|
||||
- [ ] Add migration notes to CHANGELOG
|
||||
---
|
||||
|
||||
## Risks and Mitigations
|
||||
## Summary of Required Changes
|
||||
|
||||
| Risk | Impact | Mitigation |
|
||||
|------|--------|------------|
|
||||
| Breaking change for consumers | High | Semver major version bump |
|
||||
| Import attributes not supported in older bundlers | Medium | Test with webpack, vite, rollup |
|
||||
| Some edge cases with re-exports | Low | Careful testing |
|
||||
| browser-playground uses webpack | Low | Webpack handles bundling differently |
|
||||
1. **Root package.json**: Add TypeScript override
|
||||
2. **All packages**: TypeScript 5.8.3, ts-jest 29.0.3
|
||||
3. **All tsconfigs**: node16 moduleResolution, rewriteRelativeImportExtensions
|
||||
4. **All imports**: Add .ts extensions (already done)
|
||||
5. **Build scripts**: Add post-build .d.ts fix
|
||||
6. **languageserver**: Debug test hang issue
|
||||
|
||||
## Timeline
|
||||
|
||||
1. **Wait for pending PRs** - ~1 week
|
||||
2. **TypeScript upgrade** - 1 day
|
||||
3. **Import migration** - 2-3 days
|
||||
4. **Testing & verification** - 1 day
|
||||
5. **Documentation** - 0.5 day
|
||||
|
||||
**Total: ~1-2 weeks** after dependencies are ready
|
||||
---
|
||||
|
||||
## References
|
||||
|
||||
- [TypeScript moduleResolution reference](https://www.typescriptlang.org/docs/handbook/modules/reference.html)
|
||||
- [TypeScript 5.7 rewriteRelativeImportExtensions](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-5-7.html#path-rewriting-for-relative-paths)
|
||||
- [TypeScript .d.ts extension bug #61037](https://github.com/microsoft/TypeScript/issues/61037)
|
||||
- [Node.js ESM mandatory extensions](https://nodejs.org/api/esm.html#mandatory-file-extensions)
|
||||
- [Import Attributes proposal](https://github.com/tc39/proposal-import-attributes)
|
||||
- [ts-jest ESM support](https://kulshekhar.github.io/ts-jest/docs/guides/esm-support)
|
||||
- [Community fork that works](https://github.com/boxbuild-io/actions-languageservices/commit/077fb2b58dfd2cca3d6e3df1fdf9e26e75db24ae)
|
||||
|
||||
Executable
+69
@@ -0,0 +1,69 @@
|
||||
#!/usr/bin/env node
|
||||
|
||||
/**
|
||||
* Post-build script to fix TypeScript's rewriteRelativeImportExtensions bug
|
||||
* where .d.ts files get .ts extensions instead of .js extensions.
|
||||
* See: https://github.com/microsoft/TypeScript/issues/61037
|
||||
*/
|
||||
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
|
||||
function fixDtsFile(filePath) {
|
||||
let content = fs.readFileSync(filePath, 'utf8');
|
||||
const original = content;
|
||||
|
||||
// Replace .ts extensions in import/export statements with .js
|
||||
// Matches: from "./foo.ts" or from './foo.ts'
|
||||
content = content.replace(/(from\s+["'])([^"']+)\.ts(["'])/g, '$1$2.js$3');
|
||||
|
||||
// Matches: import("./foo.ts") or import('./foo.ts')
|
||||
content = content.replace(/(import\s*\(\s*["'])([^"']+)\.ts(["']\s*\))/g, '$1$2.js$3');
|
||||
|
||||
// Matches: export * from "./foo.ts"
|
||||
content = content.replace(/(export\s+\*\s+from\s+["'])([^"']+)\.ts(["'])/g, '$1$2.js$3');
|
||||
|
||||
if (content !== original) {
|
||||
fs.writeFileSync(filePath, content, 'utf8');
|
||||
console.log(`Fixed: ${filePath}`);
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
function walkDir(dir, callback) {
|
||||
const files = fs.readdirSync(dir);
|
||||
for (const file of files) {
|
||||
const filePath = path.join(dir, file);
|
||||
const stat = fs.statSync(filePath);
|
||||
if (stat.isDirectory()) {
|
||||
walkDir(filePath, callback);
|
||||
} else if (file.endsWith('.d.ts')) {
|
||||
callback(filePath);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function main() {
|
||||
const args = process.argv.slice(2);
|
||||
if (args.length === 0) {
|
||||
console.error('Usage: fix-dts-extensions.js <dist-dir> [<dist-dir2> ...]');
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
let fixedCount = 0;
|
||||
for (const dir of args) {
|
||||
if (!fs.existsSync(dir)) {
|
||||
console.warn(`Directory not found: ${dir}`);
|
||||
continue;
|
||||
}
|
||||
walkDir(dir, (filePath) => {
|
||||
if (fixDtsFile(filePath)) {
|
||||
fixedCount++;
|
||||
}
|
||||
});
|
||||
}
|
||||
console.log(`Fixed ${fixedCount} .d.ts file(s)`);
|
||||
}
|
||||
|
||||
main();
|
||||
Reference in New Issue
Block a user