Skip to content
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

deeplyFulfilled erases type information #1257

Closed
turadg opened this issue Aug 17, 2022 · 5 comments · Fixed by #2312
Closed

deeplyFulfilled erases type information #1257

turadg opened this issue Aug 17, 2022 · 5 comments · Fixed by #2312
Assignees
Labels
kriskowal-review-2024-01 Issues that kriskowal wants to bring to the attention of the team for review as of January, 2024

Comments

@turadg
Copy link
Member

turadg commented Aug 17, 2022

https://github.com/endojs/endo/blob/master/packages/marshal/src/deeplyFulfilled.js#L36-L39

https://github.com/endojs/endo/blob/master/packages/marshal/src/types.js#L26

Because Passable is any, the type passed into deeplyFulfilled gets erased. The decision to adopt bare deeplyFulfilled in Agoric/agoric-sdk@a8db354 has erased type information there. Example impact: I just had a CI failure from a refactoring that the types would have alerted me to.

@dckc
Copy link
Contributor

dckc commented Jan 18, 2023

This week I sort of got bit by this.

The type-safe alternative, deeplyFulfilledObject, didn't work out either, though:

https://github.com/Agoric/agoric-sdk/pull/6774/commits/eb26cb200f2c68d8a636380c871090fe4468f67fb3

part of:

@erights
Copy link
Contributor

erights commented Mar 3, 2023

Not directly relevant, but worth linking to: Agoric/agoric-sdk#6816

@turadg
Copy link
Member Author

turadg commented Mar 4, 2023

May be solved by bringing these types over from sdk, https://github.com/Agoric/agoric-sdk/blob/master/packages/internal/src/utils.js#L160-L168

@kriskowal kriskowal added the kriskowal-review-2024-01 Issues that kriskowal wants to bring to the attention of the team for review as of January, 2024 label Jan 9, 2024
@erights
Copy link
Contributor

erights commented Jan 20, 2024

Would #1933 address this issue?

@erights erights self-assigned this Mar 4, 2024
@turadg
Copy link
Member Author

turadg commented Jun 9, 2024

Would #1933 address this issue?

No, because the return type still widens to Passable. We need the function to be generic on its parameters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kriskowal-review-2024-01 Issues that kriskowal wants to bring to the attention of the team for review as of January, 2024
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants