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
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