search

microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.17k stars 12.38k forks source link

[Microsoft.TypeScript.MSBuild] Incremental build is broken for .NET (Core) #50599

Open czdietrich opened 2 years ago

czdietrich commented 2 years ago

Building a project that uses Microsoft.TypeScript.MSBuild NuGet using e.g. dotnet build will compile typescript files each run even though there were no changes.

NuGet version: 4.8.2

Details

The target PreComputeCompileTypeScriptWithTSConfig will run before the actual typescript compilation and use the GenerateOutputLogs task to check whether there is already a valid cache file in the obj folder in the form of Tsc<NUMBERS>.out. Unfortunately this <NUMBERS> block is generated by using the string.GetHashCode() method on the file path of the passed tsconfig.json file. This hash code function is not stable in .NET (Core) and will be different for each new process, see also Remarks in the documentation. That's why the normal hash code of a string cannot be used to persist data.

Proposed Fix

There was a similar issue for the incremental build in the WPF repo. The fix was just to replace the hash code generation with a stable version. See also this PR.

I could not find the code of the Microsoft.TypeScript.MSBuild NuGet in this repo else I had also made a PR containing the fix.

groogiam commented 1 year ago

Is there any update on this? This is killing productivity. We have typescript complication on a project that has some long running AOT and this causes every build during the development to rebuild this expensive project. Thanks.

czdietrich commented 1 year ago

Would be nice, if this is getting fixed. The patch is trivial.

Since there was no activity on this issue we fixed it on ourselves by creating a copy of this NuGet using a stable string hashing.

czdietrich commented 1 year ago

Any chance we can just patch this issue? This is an extreme productivity stopper and just costs time and money. The fix is simple, so please let us invest these hand full of minutes and fix this in the official NuGet.

georg-eckert-zeiss commented 1 year ago

Please release a fix for this.

joj commented 1 year ago

Removed mine and assigned myself to look into this

pombredanne commented 1 year ago

@joj gentle ping, also would you know where is the source code for this? I could not find it anywhere.

joj commented 1 year ago

Thanks for the ping, this got lost in the noise for some reason. I'm creating an internal item to track it to make sure that we look into it. Sorry for that!

joj commented 1 year ago

On the source code, the typescript nuget code was never open sourced (like the rest of typescript). I can make the change, though.

pombredanne commented 1 year ago

@joj re:

On the source code, the typescript nuget code was never open sourced (like the rest of typescript). I can make the change, though.

This would be super gentle and mightily useful to have this open source! IMHO, this would have made this issue mostly a non-issue as it could have been fixed with a PR a while back :)

joj commented 1 year ago

Yeah, I see your point. Making something open source is a very intentional decision, though, and at some point we decided the nuget didn't meet the bar. I'll try to get that change soonish... I was researching and the solution to the hash code problem people are coming up with I'm not convinced about. I'll try something better (more standard).

czdietrich commented 1 year ago

Hi, is there any progress on this issue?

safacero commented 1 year ago

I fixed this today, should be on next release :D

czdietrich commented 1 year ago

Is there a prerelease NuGet which we can test?

czdietrich commented 11 months ago

There was a 5.3.0 beta release for that NuGet, recently. We tested it on our side and can confirm that it now works as expected. Thank you, the issue is now solved from my point of view, so we could close the ticket.