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

Spec review #33

Open
ljharb opened this issue Sep 2, 2019 · 3 comments

Comments

@ljharb
Copy link
Member

commented Sep 2, 2019

  • %AggregateError% is not defined (fixed in #35)
  • %AggregateErrorPrototype% should be %AggregateError.prototype% (fixed in #35)
  • Should AggregateError.prototype.errors be a non-enumerable accessor that returns an empty array?
  • [ ] AggregateError.prototype.toString: should this exist and do something different than Error.prototype.toString? (#36)
  • in AggregateError, the GetMethod call can be removed if the IterableToList call omits the method argument (fixed in #35)
  • the errors property can be set with Perform ? CreateMethodProperty(O, "errors", errorsArray) (fixed in #35)
@domenic

This comment has been minimized.

Copy link
Member

commented Sep 3, 2019

Should AggregateError.prototype.errors be a non-enumerable accessor that returns an empty array?

No, deeply-mutable prototype properties are bad news.

@ljharb

This comment has been minimized.

Copy link
Member Author

commented Sep 3, 2019

I’m asking because all the other Error own properties also have one on the prototype; the individual aggregate error types would still have their own errors data property, and because the prototype would have an accessor that returned a new array each time, it wouldn’t have the footgun you mention.

@domenic

This comment has been minimized.

Copy link
Member

commented Sep 3, 2019

I don't think that dimension of consistency is valuable here, especially if it's going to be inconsistent by virtue of being an accessor anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.