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

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 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

5 participants