-
-
Couldn't load subscription status.
- Fork 33
refactor!: split file input change event into seperate function
#129
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
Conversation
Signed-off-by: Jon Koops <jonkoops@gmail.com> BREAKING CHANGE: File input `change` events are now handled in a dedicated `fromChangeEvent()` function.
coveralls
commented
Feb 24, 2025
Pull Request Test Coverage Report for Build 13499636153Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do believe that providing a convenience function that runs though each would help users migrate smoothly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am going to rename and refactor fromEvent to something else as well, perhaps we can then evaluate adding it back and see if it is worth to keep it in a deprecated manner?
I have to do some digging as well and see how we're going to go about upgrading this dependency in the main project, so there will likely be a beta branch there as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we also check with isObject? Since we don't know what to expect at runtime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that is needed, we already have a very specific type of Event, so this value should always be correct. Of course users can pass whatever they want if they are not using TypeScript, but then they are still violating the API contract, and it is their responsibility to do this check.
What kind of change does this PR introduce?
Did you add tests for your changes?
If relevant, did you update the documentation?
Summary
This PR is a continuation of #126 and removes the handling of the
changeevent from thefromEvent()function, and moves it to a dedicatedfromChangeEvent()function purposefully made for handling this specific event.This reduces the need to handle polymorphic input and narrow down what the exact type should be in
fromEvent(), in this case also removing the need for this operation to beasync. Additionally, this allows bundlers to more effectively tree-shake the module in case the other code is never used. The JSDoc on the function itself has also been improved with additional details for the consumer.Does this PR introduce a breaking change?
Yes,
changeevents are now handled in a dedicatedfromChangeEvent()function. Users that were previously usingfromEvent()will now have to use this dedicated function instead.Other information
Not applicable.