-
Notifications
You must be signed in to change notification settings - Fork 117
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
Add alternative signatures for Model#start and Model#on that don't use var-args #263
Conversation
…e var-args The var-args in the middle of parameter lists for Model#start and Model#on make it difficult to write TypeScript type definitions for them. These backwards-compatible changes add alternative signatures that don't use var-args. 1) Model#start now supports a new array format for input paths: model.start(outPath, inPath1, inPath2, ..., fn); // Old model.start(outPath, [inPath1, inPath2, ...], fn); // New 2) Model#on now checks if the path pattern is a pre-split array, and if so, it calls the listener with a ____Event instance and the wildcard-captured segments as an array: model.on('change', 'foo.**', (captures..., value, previous, passed) => {}); // Old model.on('change', ['foo', '**'], (changeEvent, captures) => {}); // New A ChangeEvent would look like {value, previous, passed}.
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.
See comments for changes
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.
Made changes based on additional discussion in person:
- Added a
{useEventObjects: true}
option toModel#on
, which causes the callback to be invoked ascb(event, captures)
instead of the now-legacy var-argscb(captures..., [eventType], eventArgs..., passed)
. This is more explicit than argument-detection, and it enables the use-case you brought up of passing in a scoped model. - Split old code in
modelEventListener
out into a separatemodelEventListenerLegacy
function. - Because we no longer need argument-detection, I've reverted the change to take the pattern as an array of path segments. Even though we still want to move that direction longer term, it's weird to have
Model#on
be the only thing in the public API that accepts path segments.
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.
Some minor additional comments. LGTM with these resolved
The var-args in the middle of parameter lists for
Model#start
andModel#on
make it difficult to write TypeScript type definitions for them.These backwards-compatible changes add alternative signatures that don't use var-args.
Model#start
now supports a new array format for input paths:Model#on
has a new{useEventObjects: boolean}
option. If true, it calls the listener with anEvent
object and the wildcard-captured segments as an array:Once this is published, I'll update derby-site with documentation on the new signatures and event objects.