Skip to content
This repository was archived by the owner on Apr 15, 2025. It is now read-only.

brad4d Stage 3 Review feedback #364

Open
2 tasks
brad4d opened this issue Nov 15, 2022 · 9 comments
Open
2 tasks

brad4d Stage 3 Review feedback #364

brad4d opened this issue Nov 15, 2022 · 9 comments

Comments

@brad4d
Copy link

brad4d commented Nov 15, 2022

For all of these questions I searched through the issues (including closed) and the main-page README.md to find an answer before asking here. If one of these answers is there somewhere, perhaps it could be made more prominent.

  • 14.2.1 - Why allow class A extends Tuple { at all?
  • 14.2.3.7 - Why does Tuple.prototype.concat(...args) close up "holes" within the argument array-likes instead of filling in undefined values? This seems inconsistent with the behavior of Array.prototype.concat(...args).

I apologize for leaving this so late.
At least I didn't find much to comment on.

@ljharb
Copy link
Member

ljharb commented Nov 15, 2022

How would one disallow extension? Everything that ES provides can be a superclass, afaik.

@brad4d
Copy link
Author

brad4d commented Nov 15, 2022

@ljharb It seems to me that there are other globals that cause an exception if you try to use them in an extends clause.

For example:

C = class extends parseFloat {};

Generates this exception in Chrome:

VM287:1 Uncaught TypeError: Class extends value function parseFloat() { [native code] } is not a constructor or null
    at <anonymous>:1:19

I haven't yet tried to find which part of the spec specifies this behavior, but perhaps you already know?

@ljharb
Copy link
Member

ljharb commented Nov 15, 2022

Ah, yes - functions without a .prototype. However, that wouldn’t make sense when there can be instances of both.

@acutmore
Copy link
Collaborator

Why allow class A extends Tuple { at all?

One argument is consistency with other recently added types, extends BigInt is allowed in the same way. Hard to say when to start a new pattern vs follow existing conventions.

@brad4d
Copy link
Author

brad4d commented Nov 15, 2022

I don't see a strong reason to push back on the extension bullet point.
It would be nice to simply refuse to accept Record or Tuple in the extends slot, but I don't think its crucial.
The exception throw by calling super() is likely sufficient discouragement.

What about the concat() behavior bullet point, though?
Was this deviation from Array.prototype.concat() intentional?
Did I miss documentation for why this choice was made?

@nicolo-ribaudo
Copy link
Member

Did you mean that #[].concat({ 0: "a", 2: "b", length: 3 }) currently produces #["a", "b"] instead of #["a", undefined, "b"]? If yes, it's a spec bug.

@brad4d
Copy link
Author

brad4d commented Nov 16, 2022

@nicolo-ribaudo Yes, that's what I mean.

https://tc39.es/proposal-record-tuple/#sec-tuple.prototype.concat

See the loop in step 5.c.v.

If an expected numeric index does not exist, nothing is appended to the tuple.
I believe it should be appending an undefined value for that case.

@ljharb
Copy link
Member

ljharb commented Nov 16, 2022

Regarding extension, new class extends Symbol { constructor(desc) { return arguments.length > 0 ? Object(Symbol(desc)) : Object(Symbol()); } } works, and i'd expect a similar approach to work for Record and Tuple for consistency.

@joakim
Copy link

joakim commented Nov 28, 2022

Edit: I just learned that "The … constructor … is not intended to be used with the new operator or to be subclassed." For others who may not understand the reason why.

I do agree with @ljharb, sometimes consistency trumps reason. This should work, as it does in the playground:

new class extends Record { constructor(obj = {}) { return Object(Record(obj)); } }

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants