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

tsc --build --watch (with or without --incremental) touches all files causing problems for downstream tooling #46661

Closed
jfstephe opened this issue Nov 3, 2021 · 9 comments Β· Fixed by #48784
Labels
Fix Available A PR has been opened for this issue Fixed A PR has been merged for this issue Suggestion An idea for TypeScript

Comments

@jfstephe
Copy link

jfstephe commented Nov 3, 2021

Bug Report

πŸ”Ž Search Terms

touch watch timestamp

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about timestamp/touch

⏯ Playground Link

N/A

πŸ’» Code

tsc --build --watch

πŸ™ Actual behavior

Given a TSC output including:

-rw-r--r-- 1 root root 28561 Nov 3 13:25 app.js
-rw-r--r-- 1 root root 11643 Nov 3 13:25 app.js.map

Change a single file that isn't app.ts results in:

-rw-r--r-- 1 root root 28561 Nov 3 13:37 app.js
-rw-r--r-- 1 root root 11643 Nov 3 13:37 app.js.map

This completely kills downstream tools (e.g. the karma testing tool, when it is watching the output) that are watching the files for changes. Yes these could do a better job but the cause just seems so unnecessary and avoidable (especially with the --incremental option!).

πŸ™‚ Expected behavior

-rw-r--r-- 1 root root 28561 Nov 3 13:25 app.js
-rw-r--r-- 1 root root 11643 Nov 3 13:25 app.js.map

Changes to TS files don't result in any changes to any JS file that didn't have it's TS file changed

-rw-r--r-- 1 root root 28561 Nov 3 13:25 app.js
-rw-r--r-- 1 root root 11643 Nov 3 13:25 app.js.map

I've tried with and without the --incremental flag and no joy.

@andrewbranch andrewbranch added the Working as Intended The behavior described is the intended behavior; this is not a bug label Nov 3, 2021
@andrewbranch
Copy link
Member

It’s not unnecessary: 7b290fd

Update the timestamps of outputs that dont need to be written because of incremental build

This ensures that after tsbuild after incremental build of tsbuild -w doesnt result in unnecessary rebuilds

Sounds like this is used to tell future invocations of tsc that outputs are up-to-date, allowing work to be skipped.

@jfstephe
Copy link
Author

jfstephe commented Nov 3, 2021

@andrewbranch thanks for responding.

I'm not disputing that this is how it works, merely that perhaps there are better ways that don't affect downstream operations.

This issue definitely exists. tsc may be working as intended, but not as it is needed to or IMHO should.

Could the incremental data not contain information to remove the need to do this (for example)?

@andrewbranch
Copy link
Member

e.g. the karma testing tool... Yes these could do a better job

I’m curious if you’re having issues with anything besides Karma. Also what does it mean for them to be β€œcompletely killed”?

@jfstephe
Copy link
Author

jfstephe commented Nov 4, 2021

Hi, yeah I should have elaborated.

Karma watch seems to be based on timestamps, so when one typescript file changes, tsc --watch touches all files and then karma change notifications happen for ALL files (this is really, several minutes sometimes, slow).

I completely understand that karma should be better in this, however the root cause as I see it is that tsc shouldn't be touching files that it doesn't need to. There may be several other tools that have this same problem.

@andrewbranch
Copy link
Member

We’ve opened #46677 to track experiments with this. It may be a while before we can get to it, so I would encourage you to subscribe there (@typescript-bot will eventually close this issue). We’re unfortunately at a temporary shortage of people who have a deep understanding of our watch/build/incremental mode stuff, but it’s something we know we need to give more attention to when we can. I just wanted to convey that this is expected behavior for now, but we’re listening to feedback, but also you may want to investigate some workarounds in the short term. Thanks for understanding!

@jfstephe
Copy link
Author

jfstephe commented Nov 4, 2021

Awesome thanks.

@jfstephe
Copy link
Author

jfstephe commented Nov 5, 2021

Please also note that we are running on docker which is I suspect a contributing factor to the performance issue for downstream tooling.

@typescript-bot
Copy link
Collaborator

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@typescript-bot typescript-bot added the Fix Available A PR has been opened for this issue label Jun 7, 2022
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 4.8.0 milestone Jun 8, 2022
@DanielRosenwasser DanielRosenwasser added Suggestion An idea for TypeScript Fixed A PR has been merged for this issue and removed Working as Intended The behavior described is the intended behavior; this is not a bug labels Jun 8, 2022
@DanielRosenwasser
Copy link
Member

If you'd be would be willing to try out tomorrow's nightly build, we'd appreciate the feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fix Available A PR has been opened for this issue Fixed A PR has been merged for this issue Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants