Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
types(zoe): setTestJig param type optional (#9533)
closes: #XXXX refs: #9531 (comment) ## Description The implementation of `setTestJig` at https://github.com/Agoric/agoric-sdk/blob/37ec151b08de3d8e432a2599ccc532d4e72caedb/packages/zoe/src/contractFacet/zcfZygote.js#L358 treats its param as optional. The call to `setTestJig` at https://github.com/Agoric/documentation/blob/89a7dd53cd59b4008a36d23abdfa5665d1852336/snippets/tools/zcfTesterContract.js#L12 omits its parameter. The doc-comment on the type at https://github.com/Agoric/agoric-sdk/blob/37ec151b08de3d8e432a2599ccc532d4e72caedb/packages/zoe/src/contractFacet/types-ambient.d.ts#L139 explains the parameter as optional. But the type itself declares the parameter as mandatory. This initially led me at #9531 to declare the parameter as mandatory in the new `ZcfI` interface guard, but that caused the failure in the documentation repo discussed at https://github.com/Agoric/agoric-sdk/blob/37ec151b08de3d8e432a2599ccc532d4e72caedb/packages/zoe/src/contractFacet/types-ambient.d.ts#L139 . My comment in the code there and the subsequent discussion assumes that the usage at the documentation repo is what needs to be fixed. But given this other evidence, I think the static type needs to be fixed to type that parameter as optional. #9531 would then be ok as is, only needing removal of the comment indicating something is amiss. ### Security Considerations There is an existing security concern with the existence of `setTestJig` at all. But this PR does not affect that security concern at all. ### Scaling Considerations none ### Documentation Considerations This PR would make the `setTestJig` call currently in the documentation repo correct. ### Testing Considerations This problem was initially detected when testing #9531 when the guard declared the parameter as mandatory. This does reenforce the lesson that TS types are unsound by enforced guards are sound. ### Upgrade Considerations This PR is only a static change consistent with all current usage and implementation, and so should have no upgrade considerations. However, just to minimize risk, it still makes sense to hold this back till after master is snapshot for u16.
- Loading branch information