refactor(pass-style): use confirm/reject pattern in pass-style#2917
Merged
refactor(pass-style): use confirm/reject pattern in pass-style#2917
Conversation
erights
force-pushed
the
markm-use-rejector-in-passStyleOf
branch
from
cacc7c7 to
73e0b26
Compare
erights
commented
Aug 2, 2025
erights
force-pushed
the
markm-gibson-2285-rejector
branch
2 times, most recently
from
c6c560d to
14042c8
Compare
erights
force-pushed
the
markm-use-rejector-in-passStyleOf
branch
from
73e0b26 to
98354b5
Compare
erights
commented
Aug 2, 2025
1 task
erights
force-pushed
the
markm-gibson-2285-rejector
branch
from
14042c8 to
67751a5
Compare
erights
force-pushed
the
markm-use-rejector-in-passStyleOf
branch
from
98354b5 to
216d311
Compare
erights
changed the base branch from
markm-gibson-2285-rejector
to
markm-rejector-only
erights
force-pushed
the
markm-use-rejector-in-passStyleOf
branch
from
216d311 to
b0fcbb1
Compare
erights
force-pushed
the
markm-rejector-only
branch
from
20d51ab to
29d8e4a
Compare
erights
force-pushed
the
markm-use-rejector-in-passStyleOf
branch
from
b0fcbb1 to
d6ca2a7
Compare
erights
force-pushed
the
markm-use-rejector-in-passStyleOf
branch
from
d6ca2a7 to
8b6f965
Compare
erights
force-pushed
the
markm-rejector-only
branch
from
29d8e4a to
cf81497
Compare
erights
force-pushed
the
markm-use-rejector-in-passStyleOf
branch
from
8b6f965 to
f37a9c0
Compare
erights
force-pushed
the
markm-rejector-only
branch
from
cf81497 to
1b011f6
Compare
erights
force-pushed
the
markm-use-rejector-in-passStyleOf
branch
from
f37a9c0 to
3fe89c1
Compare
erights
force-pushed
the
markm-rejector-only
branch
from
1b011f6 to
5debed5
Compare
erights
force-pushed
the
markm-use-rejector-in-passStyleOf
branch
from
3fe89c1 to
16a1114
Compare
erights
force-pushed
the
markm-rejector-only
branch
from
5f3461f to
8bfbef1
Compare
erights
added a commit
that referenced
this pull request
Sep 16, 2025
Closes: #2285 Refs: #2851 #2917 #2918 #2917 #2922 ## Description Follows approximately @gibson042 's suggestions at #2285, with some minor differences I'd like @gibson042 's feedback on: - I changed the template type in the `Rejector` type to end with `=> never` rather than `=> false`. - I added a `{false}` type disjunct to the `Rejector` type - made the `reject` parameter to the `confirmFoo` functions non-optional, since it would also be supplied by the `isFoo` or `assertFoo` functions. Typically, only the latter two would be exported. The only reason I can see to make `confirmFoo`'s `reject` parameter optional is to be able to export and use it directly as the predicate, which would be an abstraction leak. As stated in #2285, the purpose is to migrate existing uses of `Checker` to use `Rejector` instead. The first real tests for that is #2917 to use Rejectors instead of Checkers in @endo/pass-style, and #2922 to do likewise for @endo/patterns. ### Security Considerations Since this PR itself is only additive, if it is correct, none. If so, then #2917 and #2922 hopefully should improve security since code using Rejectors should overall be simpler and easier to review for correctness that code using Checkers. ### Scaling Considerations Most of the point. We hope that code using Rejectors will be faster than current code using Checkers. We should measure the speedups of #2917 and #2922 over `master` baseline. ### Documentation Considerations Most code uses only `isFoo` or `assertFoo` functions, whose behavior should be identical. Only users or definers of the old `checkFoo` or the new `confirmFoo` functions need to understand the difference. Not sure where these are or should be documented. ### Testing Considerations The test included in this PR also demonstrate the `confirm/reject/is/assert` pattern that Rejectors are designed to support. ### Compatibility Considerations For this PR by itself, since it is only additive, none. Further, the design of Rejectors is intended to enable code to switch over without causing external compat considerations. The redo of #2851 will be a good first test. For any old `checkFoo` code using Checkers rather than Rejectors, if those `checkFoo` functions were exported, we can deprecate and continue to export them during the transition to further avoid compat issues. ### Upgrade Considerations None of this causes any change to data that is communicated or stored. Given that the `isFoo` and `assertFoo` functions are behaviorally identical and that any exported `checkFoo` functions continue to be exported (as deprecated), there should be no upgrade concerns.
erights
force-pushed
the
markm-use-rejector-in-passStyleOf
branch
from
16a1114 to
e31b031
Compare
erights
added a commit
that referenced
this pull request
Sep 16, 2025
Staged on #2917 Closes: #XXXX Refs: #2911 #2285 #2918 #2917 ## Description Uses the new Rejector pattern suggested by #2285 and implemented in #2923 to restructure all the check*/Checker patterns in @endo/patterns to use the confirm*/reject pattern instead. This is also a test and demonstration of the basic functionality of #2923. For this experiment, as @gibson042 expected, the code is both smaller and more readable. Should be a pure refactor, not causing any effects observable from outside @endo/patterns ### Security Considerations If it is indeed a pure refactor, none. Except of course that the code is simpler, smaller, and easier to understand and review, which helps security. ### Scaling Considerations From eyeballing these, I suspect `matches` and the non-throwing cases of `mustMatch` will be faster. But we need to measure that to see how much. This could be significant, as previous measurement have shown pattern matching to be somewhat of a bottleneck. ### Documentation Considerations The actual confim/reject pattern needs to be documented somewhere, for developers that wish to use it internally. But since the confirm* functions/methods are normally encapsulated, as they are here, normal developers who consume our APIs won't need to be aware of these changes. ### Testing Considerations No tests needed to change, ***except for some error message improvements***, raising our confidence that it is a pure refactor. ### Compatibility Considerations As a pure refactor with no change to the representation of any data, should be none. There remains the linking problem if these changes to @endo/patterns are linked with a version of @endo/errors prior to #2923. This PR does pull in the `Rejector` type exported by @endo/errors/rejector.js, which does not exist in earlier forms of @endo/errors. But these are only static types and static type imports, so it is hard to see how it could cause a dynamic problem. The ***error message improvements*** might cause some golden tests to fail, which will need fixing. However, such improvements are explicitly not considered semantic/normative, and thus the need to fix those golden tests is within the rules of the road for what we consider a "pure refactor". ### Upgrade Considerations As a pure refactor, none. --------- Co-authored-by: Richard Gibson <richard.gibson@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes: #XXXX
Refs: #2911 #2285 #2918 #2922
Description
Uses the new Rejector pattern suggested by #2285 and implemented in #2923 to restructure all the check*/Checker patterns in @endo/pass-style to use the confirm*/reject pattern instead. This is also a test and demonstration of the basic functionality of #2923.
For this experiment, as @gibson042 expected, the code is both smaller and more readable.
Should be a pure refactor, not causing any effects observable from outside @endo/pass-style
Security Considerations
If it is indeed a pure refactor, none. Except for course that the code is simpler, smaller, and easier to understand and review, which helps security.
Scaling Considerations
From eyeballing these, I suspect
passStyleOfwill be faster, but need to measure that to see how much. This could be significant, as previous measurement have consistently shownpassStyleOfto be a bottleneck.Documentation Considerations
The actual confim/reject pattern needs to be documented somewhere, for developers that wish to use it internally. But since the confirm* functions/methods normally encapsulated, as they are here, normal developers who consume our APIs won't need to be aware of these changes.
Testing Considerations
No tests needed to change, raising our confidence that it is a pure refactor.
Compatibility Considerations
As a pure refactor with no change to the representation of any data, should be none.
There remains the linking problem if these changes to @endo/patterns are linked with a version of @endo/errors prior to #2923. This PR does pull in the
Rejectortype exported by @endo/errors/rejector.js, which does not exist in earlier forms of @endo/errors. But these are only static types and static type imports, so it is hard to see how it could cause a dynamic problem.Upgrade Considerations
As a pure refactor, none.