Closed Bug 1648090 Opened 4 years ago Closed 3 years ago

Implement Ergonomic brand checks

Categories

(Core :: JavaScript Engine, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
90 Branch
Tracking Status
firefox90 --- fixed

People

(Reporter: yulia, Assigned: mgaudet)

References

(Blocks 1 open bug, )

Details

(Keywords: dev-doc-complete, Whiteboard: [DocArea=JS])

Attachments

(2 files)

Ergonomic brand check proposal just went to stage 2

Assignee: nobody → mgaudet
Status: NEW → ASSIGNED

Thanks to Nicolo Ribaudo for the insight on implementation!

Depends on D86571

We have a working implementation (as far as we know -- Test262 tests if produced could certainly show oversights.) however, my current plan is not to land the implementation while the proposal is blocked.

Once the proposal is unblocked, we will modify it to resolve the blocking factors, then land.

This proposal is now stage 3! I'm hopeful it can land and ship ASAP :-D

Note (mostly to myself): At the moment, there is no test262 feature for this, and no tests yet.

test262 tests will come with this PR: https://github.com/tc39/test262/pull/2963

Well that's exciting.

I am going to try to circle back to this in the next month or so, and having tests will be appreciated!

Attachment #9169192 - Attachment description: Bug 1648090 - Add context option for ergnomic brand checks r?jorendorff → Bug 1648090 - Add context option for ergnomic brand checks r?arai
Attachment #9169193 - Attachment description: Bug 1648090 - Implement Ergonomic brand checks r?jorendorff → Bug 1648090 - Implement Ergonomic brand checks r?arai
Blocks: 1703688
Pushed by mgaudet@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/90d11c2cfc84
Add context option for ergnomic brand checks r=arai
https://hg.mozilla.org/integration/autoland/rev/236aa8da1948
Implement Ergonomic brand checks r=arai
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch

Yay, thank you! Looking forward to v90's release.

Depends on: 1706763

Note: this may not make 90; Actually shipping is blocked on Bug 1706763, though I am hoping to address that sooner rather than later.

Blocks: 1710510
Blocks: 1710516
Blocks: 1711715
Blocks: 1714105

FF90 docs for this can be tracked through https://github.com/mdn/content/issues/5380

I understand that this lets you use in to check for the existence of a private field - or more specifically, of a valid/non-nul value. Is this also intended to allow you to check if a valid private method exists?

Flags: needinfo?(mgaudet)

(In reply to Hamish Willee from comment #12)

FF90 docs for this can be tracked through https://github.com/mdn/content/issues/5380

I understand that this lets you use in to check for the existence of a private field - or more specifically, of a valid/non-nul value. Is this also intended to allow you to check if a valid private method exists?

yes, absolutely. it checks the existence of a private field or method or accessor (NOT "valid" or "non-null").

class X {
  #a;
  #b = null;
  #c() {}
  get #d() {}
  static f(o) {
    return #a in o && #b in o && #c in o && #d in o;
  }
}
X.f(new X()) // true
X.f({}) // false
Flags: needinfo?(mgaudet)

Thanks very much. As I understand it (i.e. as it is described in the spec and examples):

  • this feature is a way to confirm that a private field/method/accessor exists ("has been declared") in an object.
  • The reason you want to do this is to have an easy way of identifying that an object belongs to this class. You can do this check and easily provide a custom exception to help users understand why they can't pass in the wrong object..
  • So for example in Matthew's Scalar example you're providing a test in add() that will ensure that only Scalar objects can be added together. If you didn't use this test and passed in a non-scalar value (well, actually a value without the private field) you'd get a less helpful error.
  • Similarly, in the comment above you provide a static method that any object can use to test "X" values.
  • The in check on private fields/methods tells you the field exists, not whether the value it has is valid or non-Null. If I do an in check on a public field doesn't that also return false on field==null? I.e. different behaviour?
  • Is there anything else being able to do this is useful for?

What I'm thinking of doing is updating https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Working_With_Private_Class_Features with a section "Checking object the object type" that explains this.

My concern is that I'm missing something else obvious about when you might choose to use this. Perhaps another way to word my concern is "does this have any other use than to check that an object is of a particular type by checking it has particular private features"?

Sorry if this is very stupid. I will be looking to document this in 2 weeks.

Flags: needinfo?(mgaudet)

Mostly this sounds very reasonable. I'm not entirely sure what the MDN policy is on documenting / elucidating the weirder or more edge case issues with features, so I'll just point out a coupe nits below related to that. Apologies if this is too deep in the mud for your purposes, and feel free to ignore it :)

(In reply to Hamish Willee from comment #14)

  • this feature is a way to confirm that a private field/method/accessor exists ("has been declared") in an object.
  • The reason you want to do this is to have an easy way of identifying that an object belongs to this class.

Private fields are sort of weird, because while 99% of the time they work like and act like fields, from the specification and committee side, they're thought of as 'associated with a given object, as if there were a weak map'; the place this is really visible is Proxies.

This mostly means from a phrasing perspective, I tend to say "an object has private field #x of class C", because private fields don't quite act like properties or regular methods (see this tweet for another oddity).

You can do this check and easily provide a custom exception to help users understand why they can't pass in the wrong object..

IMO this is the most likely use case, for improving the errors around and API.

  • The in check on private fields/methods tells you the field exists, not whether the value it has is valid or non-Null. If I do an in check on a public field doesn't that also return false on field==null? I.e. different behaviour?

nope.

js> class A { f = null } 
js> a = new A 
({f:null})
js> 'f' in a
true

The regular in operator merely checks the existence of a property, nothing about its contents. There are other operators like ?. that do that sort of checking however.

  • Is there anything else being able to do this is useful for?

Maybe ljharb has other ideas :) My imagination is fairly limited here I think

What I'm thinking of doing is updating https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Working_With_Private_Class_Features with a section "Checking object the object type" that explains this.

So with the caveat that in reality private fields and 'type' are perhaps more complicated, I suspect this would be a reasonable section to add.

My concern is that I'm missing something else obvious about when you might choose to use this. Perhaps another way to word my concern is "does this have any other use than to check that an object is of a particular type by checking it has particular private features"?

Sorry if this is very stupid. I will be looking to document this in 2 weeks.

Not stupid; I don't think you're missing too much. It's intentionally a pretty simple addition to the language, one that comes from a side effect of the strict semantics around private fields/methods: the idea that unlike a regular property access, which would simply return undefined if it wasn't there, private fields/methods throw if undefined. So to allow people to write more compact code to handle potentially missing private fields, this syntax was added.

Flags: needinfo?(mgaudet)

Thanks. Extremely helpful. I'm away until 12th and will try capture this then - probably simplistically around this edge case. Will ping for sanity check as JavaScript is scary generally, and particularly horrible in the detail :-)

FYI Docs done in https://github.com/mdn/content/pull/6786. Hopefully useful. I have tried not to bang on too much about the fact that they are useful for ergonomic brand checks. Instead it simply reflects that you can now use in for checking private feature existence. The example does show the brand check though.

You need to log in before you can comment on or make changes to this bug.