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

inconsistent watch handling of arguments for watcher #9555

Closed
mhofman opened this issue Jun 21, 2024 · 0 comments · Fixed by #9556
Closed

inconsistent watch handling of arguments for watcher #9555

mhofman opened this issue Jun 21, 2024 · 0 comments · Fixed by #9556
Assignees
Labels
bug Something isn't working

Comments

@mhofman
Copy link
Member

mhofman commented Jun 21, 2024

Describe the bug

Currently watch accepts a single optional argument in 3rd position to bind an argument for the watcher. If that argument is not provided, the watcher is still called with an undefined argument, which can trip up interface guards that do not expect it.

Furthermore watchPromise which is the ancestor of watch accepted multiple positional arguments after the watcher, creating a migration footgun.

To Reproduce

Expected behavior

Accept multiple positional arguments and respect arity.

Platform Environment

master

Additional context

Screenshots

@mhofman mhofman added the bug Something isn't working label Jun 21, 2024
@mergify mergify bot closed this as completed in #9556 Jun 22, 2024
@mergify mergify bot closed this as completed in 0af876f Jun 22, 2024
mhofman added a commit that referenced this issue Jun 22, 2024
closes: #9555

## Description

This PR updates the internal watcher exo to handle an args list, and updates the `watch` tool to check the arity and realize if a 3rd argument has been provided or not. It does not currently accept a regular rest argument like `watchPromise` does.

### Security Considerations
None

### Scaling Considerations
None

### Documentation Considerations
Updated type definition

### Testing Considerations

I didn't find any unit test coverage for this but some integration tests have shape on the watcher checking the arity.

### Upgrade Considerations
Need to settle the internal handling of arguments before we commit to them in state.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants