Skip to content

Normative: Use own properties for privateName objects#124

Merged
littledan merged 2 commits intomasterfrom
private-name-details
Jul 11, 2018
Merged

Normative: Use own properties for privateName objects#124
littledan merged 2 commits intomasterfrom
private-name-details

Conversation

@littledan
Copy link
Copy Markdown
Member

This patch changes PrivateName objects to have a null prototype
and own methods, as described in

#68 (comment)

I'm not sure whether we want to do the change, but I'm writing it up
so that we can think more carefully about whether we want to go in
this direction. This patch is the most concrete, reasonable thing
I can imagine in the direction of more integrity. It's also a relatively
small change vs the previous proposal.

Some results of the change:

  • It's not possible to effectively monkey-patch anything in particular
    to intercept private name accesses; you can only change one in
    particular which you already have access to.
  • There may be slightly more memory consuption due to the privateName
    objects having more own properties, but not that much, as the methods
    remain not bound to the receiver.

Closes #68

@littledan
Copy link
Copy Markdown
Member Author

When writing this patch, I noticed that we were missing some more basic logic: #125 . There's a conflict between these two patches, but it's easily resolvable.

spec.html Outdated

<emu-clause id="sec-private-name-type-and-objects">
<h1>PrivateName Objects</h1>
<h1>privateName Objects</h1>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this can probably stay capitalized, since they still could have been the result of a constructor (see also, Descriptor objects and Iteration Result objects, and Symbol)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's a good point. Will capitalize.

@littledan
Copy link
Copy Markdown
Member Author

We discussed this patch in the decorators call; a few people had reviewed it and were in favor of it. I plan to land the patch once I make the capitalization change @ljharb suggests and update the related documentation; I am still interested in any critical feedback you all may have.

@nicolo-ribaudo does this patch seem plausible from a Babel implementation perspective?

@nicolo-ribaudo
Copy link
Copy Markdown
Member

@nicolo-ribaudo does this patch seem plausible from a Babel implementation perspective?

Yes, it doesn't add much complexity.
How should one check if a given object is a private name given that instanceof doesn't work anymore?

@ljharb
Copy link
Copy Markdown
Member

ljharb commented Jul 11, 2018

I imagine you'd capture the get and set methods elsewhere, and check that this object had === get and set methods, and met other matching criteria?

@littledan
Copy link
Copy Markdown
Member Author

Two possibilities for detecting private names:

  • Use @@toStringTag to get what it probably is (subject to spoofing)
  • For a strong, roundabout brand check, use decorators to try to construct a class that has a field with that PrivateName, and call get on an instance :)

This patch changes PrivateName objects to have a null prototype
and own methods, as described in

#68 (comment)

I'm not sure whether we want to do the change, but I'm writing it up
so that we can think more carefully about whether we want to go in
this direction. This patch is the most concrete, reasonable thing
I can imagine in the direction of more integrity. It's also a relatively
small change vs the previous proposal.

Some results of the change:
- It's not possible to effectively monkey-patch anything in particular
  to intercept private name accesses; you can only change one in
  particular which you already have access to.
- There may be slightly more memory consuption due to the privateName
  objects having more own properties, but not that much, as the methods
  remain not bound to the receiver.

Closes #68
@littledan littledan force-pushed the private-name-details branch from 912f98a to 4fb75d1 Compare July 11, 2018 08:51
@littledan
Copy link
Copy Markdown
Member Author

Rebased (with conflicts) and fixed capitalization. Landing this PR now, but I'm still happy to hear further comments about it on this thread.

@nicolo-ribaudo
Copy link
Copy Markdown
Member

This PR makes .description writable, but its changes do not have any effect. Before this pach, it was a getter. Is this intentional?

@littledan
Copy link
Copy Markdown
Member Author

@nicolo-ribaudo I didn't really think about it; we could make it non-writable. The get function is also writable, but writing it has no effect...

nicolo-ribaudo added a commit to nicolo-ribaudo/babel that referenced this pull request Jul 11, 2018
nicolo-ribaudo added a commit to nicolo-ribaudo/babel that referenced this pull request Jul 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants