Skip to content
This repository has been archived by the owner on Jan 26, 2022. It is now read-only.

spec: update second parameter of error constructor with options bag #26

Merged
merged 13 commits into from
Mar 2, 2021

Conversation

legendecas
Copy link
Member

@legendecas legendecas commented Feb 26, 2021

Update the second parameter of the error constructor with an options bag. This helps to keep consistency between the DOMException signature and the Error constructor signature, i.e. DOMException may accept an options bag too as the second parameter and populate the cause property.

spec.emu Outdated Show resolved Hide resolved
spec.emu Outdated Show resolved Hide resolved
spec.emu Outdated Show resolved Hide resolved
spec.emu Outdated Show resolved Hide resolved
@legendecas
Copy link
Member Author

@ljharb @bakkot updated according to comments. PTAL :)

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "if options is an object and it has cause make a data property on O" pattern is repeated thrice here.

Perhaps an abstract operation that takes _O_ and _options_ that does the right thing would be appropriate? (perhaps "AddErrorCause" or something)

spec.emu Outdated Show resolved Hide resolved
spec.emu Outdated Show resolved Hide resolved
spec.emu Outdated Show resolved Hide resolved
spec.emu Outdated Show resolved Hide resolved
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks

spec.emu Outdated Show resolved Hide resolved
spec.emu Outdated Show resolved Hide resolved
spec.emu Outdated Show resolved Hide resolved
spec.emu Outdated
1. <ins>If Type(_options_) is Object and ? HasProperty(_options_, `"cause"`) is *true*, then</ins>
1. <ins>Let _cause_ be ? Get(_options_, `"cause"`)</ins>
1. <ins>Perform ! CreateErrorObjectProperty(_O_, `"cause"`, _cause_).</ins>
1. <ins>Return *undefined*.</ins>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this needed at all?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not required. I was assuming all abstract operations returning with values. If there are no such assumptions in ecma262, I'll remove this line.

spec.emu Outdated Show resolved Hide resolved
spec.emu Outdated Show resolved Hide resolved
spec.emu Outdated Show resolved Hide resolved
@hemanth hemanth requested a review from bakkot March 2, 2021 05:52
spec.emu Outdated Show resolved Hide resolved
spec.emu Outdated Show resolved Hide resolved
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants