-
-
Notifications
You must be signed in to change notification settings - Fork 433
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
Feature: Support incremental build #999
Comments
Thanks for digging in and investigating and writing this up @mc0 - where do you want to take this? |
@johnnyreilly I'm hoping to start a conversation and solicit feedback on paths forward since this might pose a significant change. I'm not against contributing/helping once a path is selected. I'm also optimistic there are other paths forward here that I didn't see. |
I think if you have repro showing why solutionBuilder.build is not working, we can take a look and come up with action. |
Details to add in: #757 We have watch API support in ts-loader behind the The plan was always to graduate this to the default behaviour, however there were occasional reports of issues (unfortunately with reliable repros) which made me nervous of ever taking that step. Perhaps we should do a breaking change that defaults it to on, that might flush out a reliable repro. If indeed there is an issue. The one problem there is Vue support. I don't think anyone has succeeded in getting the watch API working with Vue. @phry has certainly tried on the (The flag there is misnamed @mc0, do you want to propose some potential paths forward? That could prompt some useful discussion perhaps. |
@sheetalkamat I don't have reproduction steps for why solution builder's
It sounds like from the wiki[1][2][3][4], from what what Sheetal said, and the reasons above regarding SolutionBuilder, the right path forward for the status quo is to complete the switch to the watch API. The downside is that the watch API doesn't write the tsBuildInfoFile out on completion. Also, ts-loader doesn't currently work on projects. Changing those two seem a bit at odds with one another. Option A: In ts-loader: Allow users to opt-in to building projects with ts-loader instead of building files. This would require having an intermediary directory set (e.g. Option B: In TypeScript: Allow an incomplete tsBuildInfoFile to be outputted via watch API that has any files the compiler interacted with during that instance. Maybe a method named 0: https://webpack.js.org/concepts/entry-points/#object-syntax |
Thanks for putting real time and thought into this @mc0. I greatly appreciate that. I'd really like to hear back from @sheetalkamat and @andrewbranch with their thoughts. I'd like to add a few thoughts into the mix as well:
I'd love to do that. As mentioned, AFAIK there's no way as yet to support Vue users with the watch API. So we'd either probably need to keep supporting the existing non watch API mechanism for that use case. We could possibly change the default behaviour to be use the watch API with the ability to fall back to the other behaviour using a feature flag. I'd love the ability to move lock, stock and barrel but that could be a first step. Options A and B both sound interesting (option A sounds pretty bold but could have merit), I'd love to hear Sheetal and Andrew's views as well. |
Sorry if you already mentioned this somewhere, but can you elaborate on the issues with Vue? Edit: looks like maybe this PR you linked is the best place to read about it: TypeStrong/fork-ts-checker-webpack-plugin#231 |
Yup - I haven't actually tried using ts-loader with watch mode and Vue so I could be wrong. But I'm working on the basis of the experience @phry had with the fork-ts-checker-webpack-plugin. It seemed that special workarounds/ hacks had to be attempted to get the watch API to play ball with Vue due to the different filename issue. I don't know if that issue also presents with ts-loader but it seems likely. I'm not a Vue user and if there's anyone out there who would like to try ts-loader with Vue and Line 167 in bfa48c7
Looking again at the end of the linked thread, there might actually be hope in terms of a workaround. But I'm not close enough to the detail. It looks like @phry got close with a kind of approach and @WolfspiritM provided some useful feedback. But it doesn't look like things have advanced subsequently. There may actually be a way forward here but I'm not clear. @phry /@WolfspiritM if you've anything to add to this please do! 🥰 cc @yyx990803 just in case this thread proves to be relevant to the Vue CLI which (I think?) uses ts-loader |
@johnnyreilly thanks for tagging me. Looping in @sodatea who works on our CLI. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Closing as stale. Please reopen if you'd like to work on this further. |
This still seems to be an issue for me, I am using the |
I'm not totally up to speed with this, but I have an idea that I have a feeling this changed with See conversation between the marvellous @sheetalkamat and @andrewbranch here: |
Yeah, I just have a single project, no project references. |
From this it looks like you are using ts-loader in transpile only mode so there isnt going to be tsbuild info generated for that. |
Description
There has been some excellent work by @sheetalkamat and @andrewbranch on adding incremental/projectReferences API support to TypeScript [0] and ts-loader [1]. There are still some missing pieces of functionality for the incremental support in ts-loader even after a related PR was closed [2].
The primary issue is that if the base project that has Webpack in it is built with tsconfig.json containing the incremental or composite compiler options, the incremental program is never initialized and used. It's possible that this is blocked by needing support for passing in an
oldProgram
to TypeScript'slanguageService
or rewritingts-loader
to stop using thelanguageService
. I've dug into this quite a bit and thought it might be useful to pass on my thoughts.With SolutionBuilder
I've been able to locally tweak
ts-loader
to run.build()
on a solutionBuilder it creates/uses the tsBuildInfoFile as expected. Using solutionBuilder forts-loader
has downsides. Namely, if webpack has multipleentry
values for a given project, the references will be built multiple times. Adding.build()
in place of languageService would just make that problem worse.With LanguageService
Using LanguageService would require the ability to pass in an
oldProgram
(at minimum, it might need to allow a createProgram func). This seems like the most straightforward change for ts-loader but requires TypeScript changes. It is difficult to really know which API to choose at this point but it seems that the incremental support was not added to languageService.Minimal Repo
Not a repo, but a gist: https://gist.github.com/mc0/e19f69b2b26f6b612423c17722250503
0: microsoft/TypeScript#31432
1: #935
2: #913
The text was updated successfully, but these errors were encountered: