-
Notifications
You must be signed in to change notification settings - Fork 1
Fix TS 4.8+ #1
base: master
Are you sure you want to change the base?
Fix TS 4.8+ #1
Conversation
Typescript 4.8 was not working for me so as I investigated I came to conclusion it was this file causing the issue due to their recent deprecation of decorators. These changes seem to satisfy the requirements , although I wrote it on my cell phone via github.com so styling and such may now be best...
We also had problems when upgrading TS > 4.7. @falkenhawk is it possible to see if we can merge this fix? |
Thank you @bradennapier for the PR and @emiljanitzek for the ping! I somehow missed it. I'll look into it tomorrow if time allows. |
Thank you |
@bradennapier is it potentially a BC-breaking change? Should we bump typescript dependency to 4.8+? Or will it still work with older versions of typescript? 🤔 "peerDependencies": {
"typescript": ">=3.7.2"
}, |
Honestly I am not sure lol, but I would say its not a bad idea? |
@falkenhawk Upon review, I do not believe it has breaking changes, although with the significant changes with decorators coming up now may be a good time to bump the dep. Would love to get this merged if it checks out for you though! |
I've tried to build the package locally, with your changes applied and got Error: (...)packages/ts-transform-paths/src/importPathVisitor.ts(67,22): semantic error TS2339: Property 'factory' does not exist on type 'typeof ts'. 🤔
|
@falkenhawk maybe wipe node modules? Factory is the property the docs had us change to so I guess that confirms we should update peer dep |
@bradennapier sadly, it's not that easy. The codebase is built with typescript Unfortunately I don't have time to invest into this, as we have already migrated our project, where we previously used this plugin, to https://github.com/justkey007/tsc-alias (which we also had to monke-patch as it does not handle cross-dependencies between local monorepo packages...) Would you be willing to continue here to make this package build on typescript v4? Otherwise, you might be better off simply patching it with |
@falkenhawk it turns out this a regression which makes sense since it breaks a TON of ts plugins across the board. 4.9.4 should fix it so the fixes that work around the regression should become unneeded (although recommended) from my understanding. |
Unfortunately I think 5.0 is looking into supporting paths and path rewrites potentially, but I didn't read the whole feature since its a book. |
I switched to https://www.npmjs.com/package/typescript-transform-paths, just had to edit the "paths" tsconfig setting, otherwise works the same |
Typescript 4.8 was not working for me so as I investigated I came to conclusion it was this file causing the issue due to their recent deprecation of decorators.
These changes seem to satisfy the requirements , although I wrote it in bed on my cell via github.com and just got up to test it and it built our huge project (thousands of ts files in a monorepo with references and such) and seems to work.
Have only worked with ts compiler a bit though so...
I am most wary about the change from
createLiteral
tocreateStringLiteral
- I am not sure if it can be any other value as I imagine that is the string but there are a ton of things likecreateLiteralFalse
and such so may need a switch statement there ?decorators
property deprecations microsoft/TypeScript#50343