Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(internal): Use TypeScript's built-in file watcher for SDL loader #340

Merged
merged 1 commit into from
Jul 9, 2024

Conversation

kitten
Copy link
Member

@kitten kitten commented Jul 9, 2024

Resolves #339

Summary

TODO

  • Test this on Linux and Windows
  • Check default options of ts.sys.watchFile that TypeScript itself uses (as default options were broken out of the box on macOS)

This switches the SDL loader over to using ts.sys.watchFile in favour of other, similar file watchers that work around some issues. Generally, there are some conditions under which the built-in file watcher in Node fails. It's suspected that swap-writing in some editors breaks the file watcher, for example.

Instead of working around this ourselves, we can use TypeScript's implementation, which implements all known workarounds and approaches, including polling fallbacks.

Set of changes

  • Use ts.sys.watchFile in SDL file loader's watcher

@kitten kitten requested a review from JoviDeCroock July 9, 2024 08:20
Copy link

changeset-bot bot commented Jul 9, 2024

🦋 Changeset detected

Latest commit: bdc32bd

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@kitten kitten merged commit d4b80bf into main Jul 9, 2024
2 checks passed
@kitten kitten deleted the fix/watch-file branch July 9, 2024 13:31
@github-actions github-actions bot mentioned this pull request Jul 9, 2024
@mrazauskas
Copy link

It seems that ts.sys.watchFile watchers are not unrefed. Opposite to fs.watch watcher that are created with persistent: false.

The unrefed watchers do not allow process to exit. That is fine for TypeScript language service, but that is troublesome in cases where gql.tada is used as stand alone package.


I bumped into this limitation while debugging this issue: tstyche/tstyche#276

Reproduction repo can be found here: tstyche/tstyche#276 (comment)

Not sure what could be the solution. Just wanted to draw your attention.

@mrazauskas
Copy link

Generally, there are some conditions under which the built-in file watcher in Node fails. It's suspected that swap-writing in some editors breaks the file watcher, for example.

@kitten The other difference previous implementation was using the async version of fs.watch. In contrary TypeScript's watchers are wrapped around sync fs.watch. I was experimenting with both of them while implementing watch mode for TSTyche. Indeed sometimes events were missing in the async fs.watch.

I was wondering if you tried using the sync fs.watch implementation instead?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gql.tada not updating types on schema.graphql changes
3 participants