Skip to content

Conversation

@sheetalkamat
Copy link
Member

There have been cases where file change notification is reported twice by the native node file system events. We have setup many optimizations for this. eg we schedule updates instead of updating right away so if the events occur before that scheduled update, the program gets updated only once. But if the repeat event is received after we have updated event, we recreate program where the structure is used completely but we still have new copy. This means we don't emit program but there is still cost incurrent with binding and creating type checker and more over user gets notified that file change was detected and program will be rebuilt. So, from user perspective we are doing double compilation even though we didn't really emit anything. The main root cause for creating new program is that when getting new source file version, we don't read text and figure out if it has changed or not. We used to delay it to creating source files.
With this change, we will read file if the sourceFile state is unknown (that is we received the file change event). So, if text has not changed, we won't create new program, not report file change detected either.
Also had to modify how we read file for the sourceFile so that we can use cached text because now there are two places where we are reading source file text.

Fixes #51611

… to determine the version instead of delaying so that new program is not created if file contents have not changed
Copy link
Member

@jakebailey jakebailey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good to me, good baseline improvement. I am confused about readFile though.

const compilerHost: CompilerHost = {
getSourceFile: createGetSourceFile(
(fileName, encoding) => host.readFile(fileName, encoding),
(fileName, encoding) => !encoding ? compilerHost.readFile(fileName) : host.readFile(fileName, encoding),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Odd to me that this looks like this, given compilerHost just defers to host.readFile too. Is it expected that CompilerHost is missing this parameter?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we are suppose to deprecate the encoding = charSet from compilerOptions Our APIs dont have that consistent everywhere.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compilerHost.readFile is normally used for module resolution only and it doesnt support passing encoding...

jgoz added a commit to jgoz/esbuild-plugins that referenced this pull request Nov 26, 2022
jgoz added a commit to jgoz/esbuild-plugins that referenced this pull request Nov 26, 2022
* Update mcr.microsoft.com/playwright Docker tag to v1.28.1

* Bump dependencies

* Fix pnpm-lock

* Stop esbuild in watch mode

* Update TS output to account for bug

microsoft/TypeScript#51626

* Hack

* Ignore server.close errors

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: John Gozde <[email protected]>
@sheetalkamat sheetalkamat merged commit c2519bb into main Nov 29, 2022
@sheetalkamat sheetalkamat deleted the fileTextNoChange branch November 29, 2022 20:20
sheetalkamat added a commit that referenced this pull request Nov 30, 2022
* When fsEvent for change is repeated

* When trying to check if program is uptodate, read the files from disk to determine the version instead of delaying so that new program is not created if file contents have not changed

* Test for empty string change

* Fix empty string for file versioning
cbush pushed a commit to mongodb/docs-realm that referenced this pull request Jan 4, 2023
<h3>Snyk has created this PR to upgrade typescript from 4.9.3 to
4.9.4.</h3>

:information_source: Keep your dependencies up-to-date. This makes it
easier to fix existing vulnerabilities and to more quickly identify and
fix newly disclosed vulnerabilities when they affect your project.
<hr/>

- The recommended version is **1 version** ahead of your current
version.
- The recommended version was released **21 days ago**, on 2022-12-07.


<details>
<summary><b>Release notes</b></summary>
<br/>
  <details>
    <summary>Package name: <b>typescript</b></summary>
    <ul>
      <li>
<b>4.9.4</b> - <a
href="https://snyk.io/redirect/github/microsoft/TypeScript/releases/tag/v4.9.4">2022-12-07</a></br><p>For
release notes, check out the <a
href="https://devblogs.microsoft.com/typescript/announcing-typescript-4-9"
rel="nofollow">release announcement</a>.</p>
<p>For the complete list of fixed issues, check out the</p>
<ul>
<li><a
href="https://snyk.io/redirect/github/Microsoft/TypeScript/issues?utf8=%E2%9C%93&amp;q=is%3Aissue+milestone%3A%22TypeScript+4.9.4%22+is%3Aclosed+">fixed
issues query for Typescript v4.9.4</a>.</li>
</ul>
<p>Downloads are available on:</p>
<ul>
<li><a href="https://www.npmjs.com/package/typescript"
rel="nofollow">npm</a></li>
<li><a
href="https://www.nuget.org/packages/Microsoft.TypeScript.MSBuild"
rel="nofollow">NuGet package</a></li>
</ul>
<h2>Changes:</h2>
<ul>
<li><a class="commit-link" data-hovercard-type="commit"
data-hovercard-url="https://github.com/microsoft/TypeScript/commit/e2868216f637e875a74c675845625eb15dcfe9a2/hovercard"
href="https://snyk.io/redirect/github/microsoft/TypeScript/commit/e2868216f637e875a74c675845625eb15dcfe9a2"><tt>e286821</tt></a>
Bump version to 4.9.4 and LKG.</li>
<li><a class="commit-link" data-hovercard-type="commit"
data-hovercard-url="https://github.com/microsoft/TypeScript/commit/eb5419fc8d980859b98553586dfb5f40d811a745/hovercard"
href="https://snyk.io/redirect/github/microsoft/TypeScript/commit/eb5419fc8d980859b98553586dfb5f40d811a745"><tt>eb5419f</tt></a>
Cherry-pick <a class="issue-link js-issue-link" data-error-text="Failed
to load title" data-id="1470241059" data-permission-text="Title is
private" data-url="microsoft/TypeScript#51704"
data-hovercard-type="pull_request"
data-hovercard-url="/microsoft/TypeScript/pull/51704/hovercard"
href="https://snyk.io/redirect/github/microsoft/TypeScript/pull/51704">#51704</a>
to release 4.9 (<a class="issue-link js-issue-link"
data-error-text="Failed to load title" data-id="1471670926"
data-permission-text="Title is private"
data-url="microsoft/TypeScript#51712"
data-hovercard-type="pull_request"
data-hovercard-url="/microsoft/TypeScript/pull/51712/hovercard"
href="https://snyk.io/redirect/github/microsoft/TypeScript/pull/51712">#51712</a>)</li>
<li><a class="commit-link" data-hovercard-type="commit"
data-hovercard-url="https://github.com/microsoft/TypeScript/commit/b4d382b9b12460adf2da4cc0d1429cf19f8dc8be/hovercard"
href="https://snyk.io/redirect/github/microsoft/TypeScript/commit/b4d382b9b12460adf2da4cc0d1429cf19f8dc8be"><tt>b4d382b</tt></a>
Cherry-pick changes for narrowing to tagged literal types.</li>
<li><a class="commit-link" data-hovercard-type="commit"
data-hovercard-url="https://github.com/microsoft/TypeScript/commit/e7a02f43fce47e1a39259ada5460bcc33c8e98b5/hovercard"
href="https://snyk.io/redirect/github/microsoft/TypeScript/commit/e7a02f43fce47e1a39259ada5460bcc33c8e98b5"><tt>e7a02f4</tt></a>
Port of <a class="issue-link js-issue-link" data-error-text="Failed to
load title" data-id="1460570851" data-permission-text="Title is private"
data-url="microsoft/TypeScript#51626"
data-hovercard-type="pull_request"
data-hovercard-url="/microsoft/TypeScript/pull/51626/hovercard"
href="https://snyk.io/redirect/github/microsoft/TypeScript/pull/51626">#51626</a>
and <a class="issue-link js-issue-link" data-error-text="Failed to load
title" data-id="1468824634" data-permission-text="Title is private"
data-url="microsoft/TypeScript#51689"
data-hovercard-type="pull_request"
data-hovercard-url="/microsoft/TypeScript/pull/51689/hovercard"
href="https://snyk.io/redirect/github/microsoft/TypeScript/pull/51689">#51689</a>
to release-4.9 (<a class="issue-link js-issue-link"
data-error-text="Failed to load title" data-id="1460623995"
data-permission-text="Title is private"
data-url="microsoft/TypeScript#51627"
data-hovercard-type="pull_request"
data-hovercard-url="/microsoft/TypeScript/pull/51627/hovercard"
href="https://snyk.io/redirect/github/microsoft/TypeScript/pull/51627">#51627</a>)</li>
<li><a class="commit-link" data-hovercard-type="commit"
data-hovercard-url="https://github.com/microsoft/TypeScript/commit/1727912f0437a7f367d90040fc4b0b4f3efd017a/hovercard"
href="https://snyk.io/redirect/github/microsoft/TypeScript/commit/1727912f0437a7f367d90040fc4b0b4f3efd017a"><tt>1727912</tt></a>
Cherry-pick fix around <code>visitEachChild</code> to release-4.9. (<a
class="issue-link js-issue-link" data-error-text="Failed to load title"
data-id="1450676017" data-permission-text="Title is private"
data-url="microsoft/TypeScript#51544"
data-hovercard-type="pull_request"
data-hovercard-url="/microsoft/TypeScript/pull/51544/hovercard"
href="https://snyk.io/redirect/github/microsoft/TypeScript/pull/51544">#51544</a>)</li>
</ul>
<p>This list of changes was <a
href="https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_release?releaseId=117&amp;_a=release-summary"
rel="nofollow">auto generated</a>.</p>
      </li>
      <li>
<b>4.9.3</b> - <a
href="https://snyk.io/redirect/github/microsoft/TypeScript/releases/tag/v4.9.3">2022-11-15</a></br><a
href="https://snyk.io/redirect/github/microsoft/TypeScript/releases/tag/v4.9.3">
Read more </a>
      </li>
    </ul>
from <a
href="https://snyk.io/redirect/github/Microsoft/TypeScript/releases">typescript
GitHub release notes</a>
  </details>
</details>
<hr/>

**Note:** *You are seeing this because you or someone else with access
to this repository has authorized Snyk to open upgrade PRs.*

For more information: <img
src="https://api.segment.io/v1/pixel/track?data=eyJ3cml0ZUtleSI6InJyWmxZcEdHY2RyTHZsb0lYd0dUcVg4WkFRTnNCOUEwIiwiYW5vbnltb3VzSWQiOiI3M2VkNGFjNi02YTM0LTQxYzItYWEyZC04Y2JhZmFmZDI1MDciLCJldmVudCI6IlBSIHZpZXdlZCIsInByb3BlcnRpZXMiOnsicHJJZCI6IjczZWQ0YWM2LTZhMzQtNDFjMi1hYTJkLThjYmFmYWZkMjUwNyJ9fQ=="
width="0" height="0"/>

🧐 [View latest project
report](https://app.snyk.io/org/sandbox-2ba/project/852e6e4f-be96-45c8-b370-1060f5ebee55?utm_source&#x3D;github&amp;utm_medium&#x3D;referral&amp;page&#x3D;upgrade-pr)

🛠 [Adjust upgrade PR
settings](https://app.snyk.io/org/sandbox-2ba/project/852e6e4f-be96-45c8-b370-1060f5ebee55/settings/integration?utm_source&#x3D;github&amp;utm_medium&#x3D;referral&amp;page&#x3D;upgrade-pr)

🔕 [Ignore this dependency or unsubscribe from future upgrade
PRs](https://app.snyk.io/org/sandbox-2ba/project/852e6e4f-be96-45c8-b370-1060f5ebee55/settings/integration?pkg&#x3D;typescript&amp;utm_source&#x3D;github&amp;utm_medium&#x3D;referral&amp;page&#x3D;upgrade-pr#auto-dep-upgrades)

<!---
(snyk:metadata:{"prId":"73ed4ac6-6a34-41c2-aa2d-8cbafafd2507","prPublicId":"73ed4ac6-6a34-41c2-aa2d-8cbafafd2507","dependencies":[{"name":"typescript","from":"4.9.3","to":"4.9.4"}],"packageManager":"npm","type":"auto","projectUrl":"https://app.snyk.io/org/sandbox-2ba/project/852e6e4f-be96-45c8-b370-1060f5ebee55?utm_source=github&utm_medium=referral&page=upgrade-pr","projectPublicId":"852e6e4f-be96-45c8-b370-1060f5ebee55","env":"prod","prType":"upgrade","vulns":[],"issuesToFix":[],"upgrade":[],"upgradeInfo":{"versionsDiff":1,"publishedDate":"2022-12-07T23:53:20.434Z"},"templateVariants":[],"hasFixes":false,"isMajorUpgrade":false,"isBreakingChange":false,"priorityScoreList":[]})
--->

Co-authored-by: snyk-bot <[email protected]>
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Typescript watch compiles each edited file twice

5 participants