Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

fix(types): allow string for watch handlers #10396

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

Merged
posva merged 3 commits into vuejs:dev from OrkhanAlikhanov:dev
Sep 21, 2020
Merged

Conversation

@OrkhanAlikhanov
Copy link
Contributor

@OrkhanAlikhanov OrkhanAlikhanov commented Aug 14, 2019

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:

Adds missing TypeScript type info allowing watcher handler to be of string type

paparent reacted with rocket emoji

export interface WatchOptionsWithHandler<T> extends WatchOptions {
handler: WatchHandler<T>;
handler: WatchHandler<T>|string;
Copy link
Member

Choose a reason for hiding this comment

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

The WatchHandler type itself should include string as it appears in other places

Copy link
Contributor Author

@OrkhanAlikhanov OrkhanAlikhanov Aug 14, 2019

Choose a reason for hiding this comment

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

Moved it there @posva

Copy link
Member

@posva posva left a comment

Choose a reason for hiding this comment

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

We should now be able to remove the redundant| string at line 79

Copy link
Member

@posva posva left a comment

Choose a reason for hiding this comment

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

Thank you!

@posva posva changed the title (削除) Add TypeScript watch handler to be string as it is supported (削除ここまで) (追記) fix(types): allow string for watch handlers (追記ここまで) Aug 14, 2019
Copy link
Contributor Author

@posva anything on my side to get this merged in?

Copy link
Member

posva commented Sep 12, 2019

No, just waiting :)

paparent reacted with eyes emoji

@posva posva merged commit 668e1e6 into vuejs:dev Sep 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@posva posva posva approved these changes

+1 more reviewer

@darielopper darielopper darielopper approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /