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

Regarding new "incremental" flag from TS 3.4 #913

Closed
goenning opened this issue Mar 18, 2019 · 34 comments
Closed

Regarding new "incremental" flag from TS 3.4 #913

goenning opened this issue Mar 18, 2019 · 34 comments
Labels
pinned don't let probot-stale close

Comments

@goenning
Copy link

goenning commented Mar 18, 2019

Typescript 3.4 (RC) includes a new "incremental" flag: https://devblogs.microsoft.com/typescript/announcing-typescript-3-4-rc/

I've updated to 3.4 and set this flag to true.

If I do tsc -p tsconfig.json then the build cache is generated and subsequent calls are much faster.

But if I run it through webpack (which is using ts-loader), then the build cache is not generated and subsequent calls take about the same time as previously.

Does ts-loader need to be changed in order to support it?

@johnnyreilly
Copy link
Member

johnnyreilly commented Mar 18, 2019

Possibly - I haven't looked at it yet and so can't advise. If you'd like to look into it that'd be amazing!

@Jojoshua
Copy link

Jojoshua commented Mar 20, 2019

I was just about to open this issue. I also tried specifying compilerOptions: { incremental: true } but no dice.

@MadaraUchiha
Copy link

MadaraUchiha commented Mar 29, 2019

TypeScript 3.4.1 had officially landed. Can this feature be safely (or at all) used?

@johnnyreilly
Copy link
Member

johnnyreilly commented Mar 29, 2019

I suspect work may need to be done to add support. If someone wants to take a look that'd be greatly appreciated!

@radiosterne
Copy link

radiosterne commented Mar 29, 2019

Looking into it, we may be able to pass specialized incrementalHost to createProgram based on the flag.

@johnnyreilly
Copy link
Member

johnnyreilly commented Mar 29, 2019

Awesome - thanks!

@radiosterne
Copy link

radiosterne commented Mar 31, 2019

As of now, I see no clean way to use TSC's incremental :(

Every API they are using to scaffold incremental building is, sadly, non-public, if I'm not missing something.

@johnnyreilly, what's your take on our subsequent steps here? I propose opening an issue in Typescript repository in order to make these APIs public. But, of course, we can do it dirty and just use private APIs (by extending ts with our own typings) — which I would not recommend :)

@johnnyreilly
Copy link
Member

johnnyreilly commented Mar 31, 2019

@radiosterne thanks for investigating! I really appreciate it.

I'd recommend opening an issue with against the TypeScript repo to see if they're interested in making the API public facing. It's not unusual for them to release a feature like this and then "API release" it later. If memory serves, the original incremental build API was released with something like TypeScript 2.6 with the API not shipping until maybe 2.8 or so.

It's worth starting the conversation and linking back to this issue for context.

@radiosterne
Copy link

radiosterne commented Mar 31, 2019

Now that I've filed an issue with them and my head cleared a bit, I'm starting to wonder if we're gonna be able to employ this API at all.

The sole purpose of incremental flag, as I see it, is to speed up cold builds (webpack watch is covered by ts-loader already) by omitting non-changed files from compilation altogether (but still type-checking them, if they are using something from changed files).

Isn't it it our responsibility on cold build to compile each and every file, even if it's unchanged, in order to pass compilation result down to webpack for bundling? Or am I missing something here?

@MadaraUchiha
Copy link

MadaraUchiha commented Mar 31, 2019

I'm unsure of how the API functions, but is it not the case that it caches compilation results and gives you a faster lookup in case of cache hits? Without seeing the code, I'd naively expect the API to look the same for the consumer. (i.e. the cache layer being invisible)

@johnnyreilly
Copy link
Member

johnnyreilly commented Mar 31, 2019

Isn't it it our responsibility on cold build to compile each and every file, even if it's unchanged, in order to pass compilation result down to webpack for bundling? Or am I missing something here?

I haven't had chance to look into incremental yet and so you probably understand it better than I do. That said, my assumption is that incremental speeds up cold builds using data saved to disk during previous compilations. I don't see any reason that shouldn't be used by ts-loader on startup as well... (Always possible I may be misunderstanding it.)

@glen-84
Copy link

glen-84 commented Apr 3, 2019

microsoft/TypeScript#29978

@johnnyreilly
Copy link
Member

johnnyreilly commented Apr 3, 2019

It looks like APIs will be exposed with a future version of TS. When those are available we'd greatly welcome PRs that plug into this. 😁

@Sebazzz
Copy link

Sebazzz commented Apr 7, 2019

I don't see any reason that shouldn't be used by ts-loader on startup as well... (Always possible I may be misunderstanding it.)

So that startup is quicker is a good reason? Especially for scenario's where webpack is automatically invoked, for instance in ASP.NET Core scenarios.

@OneCyrus
Copy link

OneCyrus commented Apr 10, 2019

the interesting part here are composite projects/references. https://www.typescriptlang.org/docs/handbook/project-references.html

@gunn
Copy link

gunn commented May 17, 2019

More good TS 3.5 news:

TypeScript 3.5 includes several optimizations to caching how the state of the world was calculated – compiler settings, why files were looked up, where files were found, etc. [...] time rebuilding can be reduced by as much as 68% compared to TypeScript 3.4

https://devblogs.microsoft.com/typescript/announcing-typescript-3-5-rc/#speed-improvements

@johnnyreilly
Copy link
Member

johnnyreilly commented May 17, 2019

You might find this PR interesting! #935

@stale
Copy link

stale bot commented Jul 16, 2019

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.

@stale stale bot added the wontfix label Jul 16, 2019
@glen-84
Copy link

glen-84 commented Jul 16, 2019

Not stale, please label.

@stale stale bot removed the wontfix label Jul 16, 2019
@johnnyreilly johnnyreilly added the pinned don't let probot-stale close label Jul 16, 2019
@SebastianStehle
Copy link

SebastianStehle commented Aug 18, 2019

With the 3.6 RC it should be (hopefully) easy to implement now.

See: https://devblogs.microsoft.com/typescript/announcing-typescript-3-6-rc/

@johnnyreilly
Copy link
Member

johnnyreilly commented Aug 18, 2019

Feel free to have a go! This PR may have some useful info for reference: #935 (different API but you may get some useful tips from it)

@andrewbranch
Copy link
Contributor

andrewbranch commented Sep 10, 2019

#935 should fix this. (I haven’t yet experimented to see how it impacts build time via webpack, but the .tsbuildinfo files do get generated.)

@mc0
Copy link

mc0 commented Sep 11, 2019

@andrewbranch I believe that only added support for project references, not "incremental" builds. If the project references are composite, then they will be incremental but not the initial project.

@kirill-konshin
Copy link

kirill-konshin commented Sep 11, 2019

@mc0 @andrewbranch true because tsBuildInfoFile does not show up...

@andrewbranch
Copy link
Contributor

andrewbranch commented Sep 12, 2019

If the project references are composite, then they will be incremental but not the initial project.

Ah, yep, this is indeed what I meant. It’s a common pattern, but not necessary, for the initial project to have almost no files of its own, just references to other projects, so I wasn’t thinking about incremental for the root project. Since #999 is open now, we can use that to track adding incremental support for the root project. Thanks for the clarification @mc0.

@andrewbranch
Copy link
Contributor

andrewbranch commented Sep 30, 2019

@mc0 @kirill-konshin, would you be able to see if this is fixed for you via #1017? (cc @johnnyreilly)

@kirill-konshin
Copy link

kirill-konshin commented Oct 2, 2019

@andrewbranch @mc0 @johnnyreilly it seems to work as expected now. Although in #1017 people complain on performance and that build info is not emitted if transpileOnly is set to true, but I cannot confirm that, in my case everything works.

@lorenzodallavecchia
Copy link
Contributor

lorenzodallavecchia commented Oct 2, 2019

I tested some more and I can confirm that the file is being created. The needed settings are experimentalWatchApi and incremental in tsconfig.json.

Unfortunately, I am also not seeing any performance improvement: my build takes 90 s both with and without .tsbuildinfo.

Maybe the problem is that the file is being treated as a webpack output asset. This surprised me as I expected it to be treated like a collateral "meta" file.
Being an output asset also explains why people are not seeing it generated with webpack-dev-server: it actually is generated, but inside the in-memory file system that the dev server uses. Of course, this negates any performance improvement for dev-server users.

@kirill-konshin
Copy link

kirill-konshin commented Oct 2, 2019

In fact, I was not using experimentalWatchApi but was using incremental and still got the build info from Webpack Dev Server.

I even got TS5023: Unknown compiler option 'experimentalWatchApi', I am using TS 3.6.3.

Strange thing is that I'm not getting build info on Webpack build (using webpack-cli)... Yet subsequent builds are much faster.

Performance wise I don't see any improvements.

@kirill-konshin
Copy link

kirill-konshin commented Oct 2, 2019

Forgot to mention, in my setup I have transpileOnly and ForkTsCheckerWebpackPlugin.

@lorenzodallavecchia
Copy link
Contributor

lorenzodallavecchia commented Oct 2, 2019

I even got TS5023: Unknown compiler option 'experimentalWatchApi', I am using TS 3.6.3.

@kirill-konshin if you are getting that error you are probably placing the experimentalWatchApi flag in the wrong place. It should not be passed to TypeScript (i.e. not in compilerOptions).
For example:

{
  test: /\.ts$/,
  exclude: /node_modules/,
  loader: "ts-loader",
  options: {
    compilerOptions: {
      target: "es5",
      incremental: true,  // this could also be in tsconfig.json directly
    },
    experimentalWatchApi: true,,  // this enables .tsbuildinfo in the loader
  },
},

@OneCyrus
Copy link

OneCyrus commented Oct 2, 2019

Being an output asset also explains why people are not seeing it generated with webpack-dev-server: it actually is generated, but inside the in-memory file system that the dev server uses. Of course, this negates any performance improvement for dev-server users.

that clears things up. also the strange effects with babel and so. but in the end the question is what this tsbuildinfo really improves? there doesn't seem to be much advanced information in this file. basically just the import file resolutions. i thought it would save like the complete type graph in there so it can bascially start without initialization.

@kirill-konshin
Copy link

kirill-konshin commented Oct 2, 2019

@lorenzodallavecchia yes, you're right! Thanks.

@kirill-konshin
Copy link

kirill-konshin commented Oct 2, 2019

I also noted weird behavior that transpileOnly: true and ForkTsCheckerWebpackPlugin combination fails when upstream is not built. Upstream eventually becomes built but looks like plugin is unaware of that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pinned don't let probot-stale close
Projects
None yet
Development

No branches or pull requests