-
Notifications
You must be signed in to change notification settings - Fork 82
Description
What is the Problem Being Solved?
#2284 demonstrated that there is a relatively small but still significant performance cost to creating reject = !!check && ((T, ...subs) => check(false, X(T, ...subs))) functions that will only be used in failure paths, and so moved that definition into a new CX function and updated many sites to use that:
- reject && reject`…`
+ !!check && CX(check)`…`However, this has drawbacks of its own with respect to comprehensibility and readability, especially when prettier introduces line breaks like
(
!!check &&
CX(
check,
)`…`
)Description of the Design
I think we'd be better off replacing (cond: boolean, details?: Details | undefined) => boolean "Checker" parameters with optional (template: TemplateStringsArray | string[], ...args: any[]) => false "Rejector" parameters, recovering the old local pattern but without local function creation.
And per #2285 (comment) , we'll also want to rename internal functions for indicating the progress of this transition.
-const checkSomething = (candidate, check = undefined) => {
+const confirmSomething = (candidate, reject = false) => {
return (
isOk(candidate) ||
- (!!check && CX(check)`…`)
+ (reject && reject`…`)
);
};And at the highest possible level, we'd replace use of x => x checkers with undefined rejectors and assertChecker with assert.Fail.
- checkSomething(candidate, assertChecker)
+ confirmSomething(candidate, Fail)Security Considerations
None that I can see.
Scaling Considerations
Negligible, but the new pattern might be slightly faster.
Test Plan
n/a, this is behavior-preserving refactoring.
Compatibility Considerations
assertChecker is exported by the pass-style package, and used by at least the patterns package, which also makes use of checkers so should be updated similarly. But I think all such use is internal for supporting {is,assert}$Type package exports (e.g., isKey and assertKey).
Upgrade Considerations
If any exports do expose a Checker parameter, we'll need to consider a deprecation strategy.