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

Presences should not have ownProperties #792

Closed
michaelfig opened this issue Mar 27, 2020 · 9 comments
Closed

Presences should not have ownProperties #792

michaelfig opened this issue Mar 27, 2020 · 9 comments
Assignees
Labels
SwingSet package: SwingSet

Comments

@michaelfig
Copy link
Member

Currently, if you pass an empty object across SwingSet vats, it is marshalled as a Presence (or a Device), and has a .toString ownProperty added to it. This makes for a surprise on the other end, where an ownProperty is not expected. This has bitten us in several cases where an empty object is a desirable feature of an interface that takes 0 or more ownProperties.

If instead we set the presence's prototype to have the .toString then it won't have any .keys() or .getOwnPropertyNames(), so it will be handled correctly as an empty object!

@warner I'll work out a PR for this.

@michaelfig michaelfig added the SwingSet package: SwingSet label Mar 27, 2020
@michaelfig michaelfig self-assigned this Mar 27, 2020
@erights
Copy link
Member

erights commented Mar 27, 2020

Curious where we're encountering this? Where do we look at the own properties of a presence?

@michaelfig
Copy link
Member Author

If we do something like E(zoe).redeem(invite, {}, {}) intending to pass an empty offer and payment, we actually end up with a presence that has a toString property which Zoe then rejects as an invalid keyword.

@michaelfig
Copy link
Member Author

michaelfig commented Mar 27, 2020

I have a better solution using a symbol registered with Symbol.for(...) so that it shows up in debug dumps (and the REPL can intercept it) but not as an ownPropertyName.

@warner
Copy link
Member

warner commented Mar 27, 2020

@erights are you ok with that? I'm all on board with empty records showing up as empty records. And I'm still happy with our decision to represent empty records as pass-by-reference marker objects (which arrive as Presences). And I'm happy to to have Presences lose their properties, as long as they retain debugability.

The thing that feels weird to me is to pass a data record in and get a Presence out. If Alice sends a record to Bob, and Bob sends it back, and Alice does === on it, she'll get true if the record was empty and false otherwise.

OTOH, if the recipient is expecting a data record, then they'll never think to do an identity check on it, and they'll never notice that the thing they received was pass-by-reference rather than pass-by-copy. So practically speaking, it could be just fine.

@erights
Copy link
Member

erights commented Mar 27, 2020

I have the same discomfort and would like to find a better solution. Let's talk tomorrow.

@michaelfig
Copy link
Member Author

Maybe we should choose to marshal empty records as pass-by-copy, and have an explicit makeHandle() helper, whose entire job is to create a record that has at least one method on it.

That would help satisfy the Principle of Least Suprise.

@erights
Copy link
Member

erights commented Mar 28, 2020

I like that direction.

@erights
Copy link
Member

erights commented Mar 29, 2020

Close in favor of #804 ?

@michaelfig
Copy link
Member Author

Close in favor of #804 ?

I'll close this one when the #793 lands (which will not close #804).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SwingSet package: SwingSet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants