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

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

Open
czdietrich opened this issue Sep 1, 2022 · 14 comments
Open
Assignees
Labels
Rescheduled This issue was previously scheduled to an earlier milestone Visual Studio Integration with Visual Studio

Comments

@czdietrich
Copy link

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.

@jakebailey jakebailey added the Visual Studio Integration with Visual Studio label Sep 2, 2022
@jakebailey jakebailey added this to the Backlog milestone Sep 2, 2022
@groogiam
Copy link

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
Copy link
Author

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
Copy link
Author

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

Please release a fix for this.

@joj joj assigned joj and unassigned minestarks Mar 9, 2023
@joj
Copy link
Member

joj commented Mar 9, 2023

Removed mine and assigned myself to look into this

@pombredanne
Copy link

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

@joj
Copy link
Member

joj commented May 23, 2023

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
Copy link
Member

joj commented May 23, 2023

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

@pombredanne
Copy link

@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
Copy link
Member

joj commented May 29, 2023

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
Copy link
Author

Hi, is there any progress on this issue?

@safacero
Copy link
Member

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

@czdietrich
Copy link
Author

Is there a prerelease NuGet which we can test?

@czdietrich
Copy link
Author

czdietrich commented Oct 5, 2023

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.

@RyanCavanaugh RyanCavanaugh added the Rescheduled This issue was previously scheduled to an earlier milestone label Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Rescheduled This issue was previously scheduled to an earlier milestone Visual Studio Integration with Visual Studio
Projects
None yet
Development

No branches or pull requests