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

Make closed private #1357

Open
benmccann opened this issue Sep 18, 2024 · 1 comment
Open

Make closed private #1357

benmccann opened this issue Sep 18, 2024 · 1 comment

Comments

@benmccann
Copy link
Contributor

benmccann commented Sep 18, 2024

Describe the bug

In v3 close() returned Promise<void>. #1356 was just merged which turns it to Promise<void> | undefined, but this doesn't feel like the correct solution to me. The only way it can return undefined is if the user does watcher.closed = true followed by watcher.close().

Expected behavior

I don't think the user should be able to directly set watcher.closed as it skips all of the other cleanup. It also makes migration from v3 to v4 more difficult by changing the type signature of close()

Additional context

See #1356 and the issue it closed #1355 for more details. There was an unresolved question about this on the PR #1356 (comment), but unfortunately I didn't have a chance to chime in.

@43081j
Copy link
Collaborator

43081j commented Sep 19, 2024

Whatever the internal logic of it, we should return a promise I think

Returning undefined right now is like leaking the underlying internal logic. It should be hidden away

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

No branches or pull requests

2 participants