Fix #266: restore in TS 5.6. #270
Conversation
Failed to compile `transform-typescript-paths` due to many TypeScript compilation errors. Therefore, don't know whether this PR is valid or not. Hope your strict your review @nonara. Instead, tested on my project manually editing the JS file in the `node_modules/typescript-transform-paths` directory.
|
Thanks for contributing @samchon, it seems the build is failing, and I'm concerned about breaking compatibility with typescript <5.6, so we probably need to add some tests to make sure older typescript versions continue working. |
|
indeed this only requires renaming a single fn name and it breaks the core release of typescript - originalSourceFile: tsInstance.getOriginalSourceFile(sourceFile),
+ originalSourceFile: tsInstance.getSourceFileOfNode(sourceFile),made my project compile -- worst case perhaps originalSourceFile: typeof tsInstance.getSourceFileOfNode === 'function' ? tsInstance.getSourceFileOfNode(sourceFile) : tsInstance.getOriginalSourceFile(sourceFile)ah it does seem entirely type based - id be surprised if it didnt require a major change due to them removing a bunch of stuff in 5.6 via knip |
| @@ -135,9 +135,9 @@ export default function transformer( | |||
| ...tsTransformPathsContext, | |||
| sourceFile, | |||
| isDeclarationFile: sourceFile.isDeclarationFile, | |||
| originalSourceFile: (<typeof ts>tsInstance).getOriginalSourceFile(sourceFile), | |||
| originalSourceFile: tsInstance.getSourceFileOfNode(sourceFile), | |||
There was a problem hiding this comment.
I wonder if, for backward compatibility, it would make more sense to do something like this?
originalSourceFile: (<typeof ts>tsInstance).getSourceFileOfNode
? (<typeof ts>tsInstance).getSourceFileOfNode(sourceFile)
: (<typeof ts>tsInstance).getOriginalSourceFile(sourceFile),
That way, we use the new method if it exists, or fall back to the old one.
There was a problem hiding this comment.
Actually, the newly suggested method here (getSourceFileOfNode) is also marked as @internal. I have a PR that uses public methods and adds test coverage here: #301
This PR restores compatibility with TypeScript 5.6. It also updates the test suite to be more resilient and explicitly tests against TypeScript 5.5 and 5.6. I was surprised that TypeScript would break this without releasing a major version. However, if you look at the `getOriginalSourceFile` method we were using, it was marked as `@internal` in the JSDoc: https://github.com/microsoft/TypeScript/blob/c8a7d589e647e19c94150d9892909f3aa93e48eb/src/compiler/utilities.ts#L5416-L5419 Because it was marked as internal, they probably reserved the right to remove this method without a breaking version bump. The other PR to fix this issue (#270) suggested using `getSourceFileOfNode`, but that's also marked as `@internal`: https://github.com/microsoft/TypeScript/blob/11b2930fa2c9f73b0ffb725a9715b8d3c4121bbc/src/compiler/utilities.ts#L965-L975. Therefore, instead, I updated this code to use `getParseTreeNode` and `isSourceFile`, which are both exposed publicly and not marked as `internal`: https://github.com/microsoft/TypeScript/blob/11b2930fa2c9f73b0ffb725a9715b8d3c4121bbc/src/compiler/utilitiesPublic.ts#L803-L809 / https://github.com/microsoft/TypeScript/blob/11b2930fa2c9f73b0ffb725a9715b8d3c4121bbc/src/compiler/factory/nodeTests.ts#L1013-L1016 By using these methods, we should be more resilient to internal refactoring. This PR supersedes #270. Fixes #266
|
This PR can be closed in favor of the merged PR #301 |
Failed to compile
transform-typescript-pathsdue to many TypeScript compilation errors. Therefore, don't know whether this PR is valid or not. Hope your strict your review @nonara.Instead, tested on my project manually editing the JS file in the
node_modules/typescript-transform-pathsdirectory.