Low urgency. This is a minor reduction of tech debt.
https://github.com/Agoric/agoric-sdk/pull/5993 introduced deeplyFulfilledObject to work around the deficiencies in the typing of deeplyFulfilled explained at #1257 . This PR fixes those deficiencies. This does not necessary make deeplyFulfilledObject unneeded, but it should enable many callers of deeplyFulfilledObject to call deeplyFulfilled instead without loss of type fidelity.
Note: the code in this first commit probably does not use TS typing well, resulting in many internal @ts-expect-error directives in the implementation. This is because I still don't really understand what I'm doing with complex TS types. Reviewers, help appreciated.
But IIUC, the external type should now be as good as deeplyFulfilledObject. Hopefully, the only typing flaws are encapsulated by this implementation. Yes?
Security Considerations
More accurate typing helps security. More precise but inaccurate unsound typing hurts security. Our security practices are already adapted to this dilemma, and this PR should not have much effect.
Scaling Considerations
none
Documentation Considerations
It would be nice if this PR eventually let us drop the need to explain deeplyFulfilledObject and why it co-exists with deeplyFulfilled
Testing Considerations
[ ] since this is all about changing static types, I need to write some static type tests.
Even though the code itself is also a bit different, these differences are with high confidence a pure refactor, so the existing dynamic tests should adequately test that.
Compatibility Considerations
Assuming that these changes are a pure refactor with no dynamic behavior changes, there should be no dynamic compat issues.
The tighter typing of deeplyFulfilled does raise the possibility that some call sites will no longer pass static type checks. But no such problem appears within the PR's CI, and therefore within the endo repo.
Upgrade Considerations
none.
[ ] Include *BREAKING*: in the commit message with migration instructions for any breaking change.
Closes: #1257 Refs: https://github.com/Agoric/agoric-sdk/pull/5993 https://github.com/Agoric/agoric-sdk/pull/6816 https://github.com/endojs/endo/pull/1455
Description
Low urgency. This is a minor reduction of tech debt.
https://github.com/Agoric/agoric-sdk/pull/5993 introduced
deeplyFulfilledObjectto work around the deficiencies in the typing ofdeeplyFulfilledexplained at #1257 . This PR fixes those deficiencies. This does not necessary makedeeplyFulfilledObjectunneeded, but it should enable many callers ofdeeplyFulfilledObjectto calldeeplyFulfilledinstead without loss of type fidelity.Note: the code in this first commit probably does not use TS typing well, resulting in many internal
@ts-expect-errordirectives in the implementation. This is because I still don't really understand what I'm doing with complex TS types. Reviewers, help appreciated.But IIUC, the external type should now be as good as
deeplyFulfilledObject. Hopefully, the only typing flaws are encapsulated by this implementation. Yes?Security Considerations
More accurate typing helps security. More precise but inaccurate unsound typing hurts security. Our security practices are already adapted to this dilemma, and this PR should not have much effect.
Scaling Considerations
none
Documentation Considerations
It would be nice if this PR eventually let us drop the need to explain
deeplyFulfilledObjectand why it co-exists withdeeplyFulfilledTesting Considerations
Even though the code itself is also a bit different, these differences are with high confidence a pure refactor, so the existing dynamic tests should adequately test that.
Compatibility Considerations
Assuming that these changes are a pure refactor with no dynamic behavior changes, there should be no dynamic compat issues.
The tighter typing of
deeplyFulfilleddoes raise the possibility that some call sites will no longer pass static type checks. But no such problem appears within the PR's CI, and therefore within the endo repo.Upgrade Considerations
none.
*BREAKING*:in the commit message with migration instructions for any breaking change.NEWS.mdfor user-facing changes.