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

fix: separate error emit args from regular emit args #1397

Merged
merged 2 commits into from
Dec 18, 2024
Merged

Conversation

43081j
Copy link
Collaborator

@43081j 43081j commented Dec 18, 2024

Separates the error event arguments from the regular event arguments.

Before, we had a merged tuple of [Path | Error, Stats?] to account for
the fact that some events can be [Path, Stats?] and some can be
[Error, Stats?].

However, this results in incorrect types since nothing will ever pass
Path | Error as a type (only one or the other).

This separates them and uses a union instead, such that event handlers
other than error only ever have a Path.

The `unlink` and `unlinkDir` events both have known types, so this adds
them to the known event map.
Separates the error event arguments from the regular event arguments.

Before, we had a merged tuple of `[Path | Error, Stats?]` to account for
the fact that some events can be `[Path, Stats?]` and some can be
`[Error, Stats?]`.

However, this results in incorrect types since nothing will ever pass
`Path | Error` as a type (only one or the other).

This separates them and uses a union instead, such that event handlers
other than `error` only ever have a `Path`.
@43081j 43081j changed the title fix: strongly type unlink events fix: separate error emit args from regular emit args Dec 18, 2024
@paulmillr paulmillr merged commit 9470932 into main Dec 18, 2024
20 checks passed
@43081j 43081j deleted the unlink-types branch December 18, 2024 22:23
@JounQin
Copy link

JounQin commented Dec 19, 2024

I got the following error after upgrading to v4.0.3 when using "@types/node": "~18.18", upgrading to ~18.19 fixes it.

src/cli/index.ts:87:20 - error TS2576: Property 'on' does not exist on type 'FSWatcher'. Did you mean to access the static member 'FSWatcher.on' instead?

87         cliWatcher.on('all', async (eventName, filepath) => {
                      ~~

src/cli/index.ts:87:37 - error TS7006: Parameter 'eventName' implicitly has an 'any' type.

87         cliWatcher.on('all', async (eventName, filepath) => {
                                       ~~~~~~~~~

src/cli/index.ts:87:48 - error TS7006: Parameter 'filepath' implicitly has an 'any' type.

87         cliWatcher.on('all', async (eventName, filepath) => {
                                                  ~~~~~~~~

@43081j
Copy link
Collaborator Author

43081j commented Dec 19, 2024

we haven't changed anything around that as far as i can tell. are you sure it didn't already error in 4.0.2 with @types/node@18.18?

and you just coincidentally upgraded both at the same time?

otherwise seems pretty strange as this PR didn't touch what node types we use

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

Successfully merging this pull request may close these issues.

3 participants