Skip to content
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

Upcoming breaking changes in TS 5.0 #83

Closed
robert-w-gries opened this issue Nov 22, 2022 · 8 comments · Fixed by #96
Closed

Upcoming breaking changes in TS 5.0 #83

robert-w-gries opened this issue Nov 22, 2022 · 8 comments · Fixed by #96

Comments

@robert-w-gries
Copy link

Hello, I've been reading about an upcoming change in Tyepscript 5.0 that looks like it will break this package. The change can be found here: microsoft/TypeScript#51387

The PR transformed the typescript source code from using namespaces to modules. I believe this will be a breaking change for this tool as pointed out in the issue:

The output files have changed significantly; if you are patching TypeScript, you will definitely need to change your patches.

I tested the 11/21 nightly by installing typescript@next and errored while trying to run npx ts-patch install. This is the result of npx ts-patch check:

$ npx ts-patch check
Checking TypeScript v5.0.0-dev.20221121 installation in D:\Projects\my_app\node_modules\typescript

[!] [FileNotFound]: Could not find file D:\Projects\my_app\node_modules\typescript\lib\typescriptServices.js.

That file was removed as part of the above linked PR. I removed it from the list of tsc source files and still got an error while running the check command:

$ npx ts-patch check
Checking TypeScript v5.0.0-dev.20221121 installation in D:\Projects\my_app\node_modules\typescript

[-] tsc.js is not patched and cannot be patched!
[-] tsserverlibrary.js is not patched and cannot be patched!
[-] typescript.js is not patched and cannot be patched!
[-] tsserver.js is not patched and cannot be patched!
@nonara
Copy link
Owner

nonara commented Nov 22, 2022

Thanks for the report Robert!

I assumed that would be the case. The author of the latest changes let me know in another repo I maintain.

We have a bit of time before a release. The release schedule is here:

I was planning to wait until RC, as it has the least chance of requiring anything be redone.

Generally speaking, though, it's fairly easy make updates for new releases. If you want to submit a PR in the meantime to use it with nightly, I'd be glad to help get it through!

@robert-w-gries
Copy link
Author

Glad to hear you’re aware and have a plan of action!

I don’t necessarily need to use ts-patch with nightly typescript. I was more curious to see how the lib files changed and how it would affect ts-patch.

Apologies if this is too off topic, but I’ve been experimenting with a local ts-patch fork that uses jscodeshift to patch typescript’s lib files. It’s an experiment in supporting custom patches as I’m interested in patching the typescript module resolution system and managing the patch using ts-patch. This is partly why I was interested in seeing how 5.0 would impact the tool. Let me know if you’d like to spin this off into the discussion tab, I’d be interested to hear your thoughts on how custom patches should work

@nonara
Copy link
Owner

nonara commented Nov 30, 2022

@robert-w-gries Sorry for the delay in getting back. Busy week!

We're starting a discussion on Slack regarding doing the new version of ts-patch, which clears up a lot of the current issues and also allows creating custom patches to TS itself as part of a plugin library. Sounds like your interests would align as well!

Would you be interested in joining the discussion?

@robert-w-gries
Copy link
Author

Yes I’d definitely be interested!

@samchon
Copy link
Contributor

samchon commented Feb 17, 2023

TS 5.0 has been changed target build option from es5 to es2018, and it invalidates key logic of ts-patch.

Wrote an issue on TypeScript repo, but I think ts-patch needs to ready for that - @nonara

microsoft/TypeScript#52826

@jakebailey
Copy link

@samchon FWIW this problem has nothing to do with the target being changed. The original error occured becuase typescriptServices.js no longer exists in the typescript package, as it was identical to typescript.js and wasting space, so has been removed in TS 5.0 (whose major bump allowed for a slew of breaking changes).

@nonara
Copy link
Owner

nonara commented Feb 17, 2023

@jakebailey Thank you for the added detail!

Looks like it's about time for me to look at adding support. I was wondering if you had any thoughts on an approach with the new version. (Note: I haven't looked at the latest TS source, so I'm not sure how the build process works yet)

If memory serves, we originally discovered that tsc.js is compiled with a more limited library set, which made transformers not have access to the full range of functionality. To fix that, we were able to slice off the top of typescript.js and splice it over the bottom of tsc.js, which worked well. (see below diagram)

I assume with the new tsc.js, we'll also need to augment it to have full functionality. What I'm hoping to determine is a viable approach to this with the new builds. Would be grateful for any info you can share on the new build process before I dig in!

image

@jakebailey
Copy link

The new build is effectively just "run esbuild on each entrypoint". esbuild can arbitrarily remove dead code, so it's not exactly cut and dry what will or will not be present in tsc (as it doesn't export anything, so the dead code elimination is very, very effective). This poses a problem for patching tsc in particular, because it's possible you'll need code that has been deleted.

This does match your picture above in that the middle one is smaller. But, as the two bundles are very different (typescript.js exports something), copy and pasting between them won't be very feasible and so I would doubt you can tack stuff on the end and get something working.

It's important to note that now that TS is modules, it isn't just a bunch of namespace functions pushing onto a common ts object anymore; trying to combine two unrelated files definitely won't do the right thing (though, probably wouldn't have done the right thing before either).

I think I'd just need to try and understand everything that this package wants to accomplish before trying to recommend anything; I'd say to try out the yarn-style patch approach, but I'm not sure if you're trying to come up with arbitrary patches or what.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants