endojs / endo

Endo is a distributed secure JavaScript sandbox, based on SES

Apache License 2.0

829 stars 72 forks source link

1488 Passable (REVERTED in #1961) #1933

Closed turadg closed 10 months ago

turadg commented 10 months ago

closes: #1488

Description

Makes Passable a generic type instead of any.
Defines overloads for passStyleOf to return the actual style.
Defines the Key in terms of Passable
Makes a ton of fixes and suppressions in places that relied on the previous any's

Security Considerations

n/a

Scaling Considerations

n/a

Documentation Considerations

Better types are better documentation.

Testing Considerations

I improved the test coverage. Additional suggestions welcomed.

Upgrade Considerations

These changes may cause type errors downstream. I don't think we should call those breaking change à la semver because they don't affect the runtime.

mhofman commented 10 months ago

I am only following from a distance, but I believe you can have circular type definitions in .d.ts files, but not in JSDoc

turadg commented 10 months ago

I am only following from a distance, but I believe you can have circular type definitions in .d.ts files, but not in JSDoc

I'm not sure about that but I do know the solution is more complicated than changing to .ts. Even with .ts,

export type CopyArray<T extends Passable = Passable> = T[];

causes

Type alias 'PassByCopy' circularly references itself.

erights commented 10 months ago

Attn @michaelfig re https://github.com/endojs/endo/pull/1933#issuecomment-1885583987 circular types mysteries

dckc commented 10 months ago

1952 shows how to address the circular type issues, I think. I discussed it with @turadg today. It's not clear how much work it would be to finish that direction.

It's based on earlier ocapn work, but at the time I learned tsc's limitations around self-referential types, I neglected to take good notes.

@mhofman gave a particularly useful pointer earlier today:

https://github.com/microsoft/TypeScript/pull/33050

turadg commented 10 months ago

1952 shows how to address the circular type issues, I think. I discussed it with @turadg today. It's not clear how much work it would be to finish that direction.

I think that solved it, @dckc . PTAL 5b8b349 9aac68e.

erights commented 9 months ago

@turadg @kriskowal all,

Now that the endo-agoric-sdk sync is behind us, I would love to see this PR move forward again! What is needed?

At https://github.com/Agoric/agoric-sdk/pull/8771 I have a PR of postponed changes on the agoric-sdk side that were waiting on that sync. Would that one help or hurt the effort to move forward on this PR?

turadg commented 9 months ago

What is needed?

In Endo, a PR re-applying these changes.

And in agoric-sdk a PR that integrates with that PR, demonstrating agoric-sdk CI passing. That way when land the Endo PR we can follow up quickly with a release and make the SDK PR the basis of an Endo upgrade.

I've started in

https://github.com/Agoric/agoric-sdk/pull/8774

At Agoric/agoric-sdk#8771 I have a PR of postponed changes on the agoric-sdk side that were waiting on that sync. Would that one help or hurt the effort to move forward on this PR?

I'm not sure. I think 8771 should be prioritized, though, because it doesn't require an Endo release. I'll try to get 8774 passing and I expect by the time it's R4R that master will have 8771 so I'll solve whatever else comes up.