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

Record.isRecord returning true for Record Exotic Objects #333

Closed
acutmore opened this issue Jul 25, 2022 · 3 comments
Closed

Record.isRecord returning true for Record Exotic Objects #333

acutmore opened this issue Jul 25, 2022 · 3 comments

Comments

@acutmore
Copy link
Collaborator

Issue raised during the July 2022 TC39 plenary by @waldemarhorwat .

In the current spec: Record.isRecord(Object(#{})) returns true, because for objects it checks the presence of the [[RecordData]] internal slot.

This means that there are an infinite number of things for which Record.isRecord would return true, that can all have the same content (same properties, with same values) yet are not strictly equal to each other.

If code is branching using Record.isRecord it possible that they are doing so because they are expecting a successful value to subsequently behave as a record (value based equality) - perhaps they plan to use it as a key in a Map. They may not be aware that this predicate also returns true for the wrapper object.

@acutmore
Copy link
Collaborator Author

A potential solution raised during the meeting was to remove Record.isRecord and instead have Record.isRecordWrapper that would return false for a primitive record, and only return true for the Record Exotic Object.

A potential risk to this, IMO, is that a static method is highly visible to developers when they first start learning the language.

IDE autocomplete for 'Record.isRecordWrapper' after typing Record dot

Potentially creating a 'forcing function' of learning about wrappers sooner than would usually be done, increasing cognitive load. It may also be confusing that there isn't an equivalent Number.isNumberWrapper.

An alternative approach would be to address 'brand checking' of records as a separate proposal that is exploring a more consistent approach across all the primitives. For example as is done in the current pattern-matching spec of adding Boolean[@@matcher] and Number[@@matcher] etc.

@rricard
Copy link
Member

rricard commented Jul 25, 2022

Alternatively, for Tuple, brand checking could be done with Tuple.prototype.valueOf, unfortunately we don't have a Record prototype to do the same there. We could propose similar functionality on the Record namespace through something like Record.recordValue.

@rricard
Copy link
Member

rricard commented Jul 25, 2022

This solution could also enable what is discussed in #329 (comment)

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

No branches or pull requests

2 participants