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

Updates for stage 2 spec review #77

Merged
merged 4 commits into from
Feb 22, 2022

Conversation

acutmore
Copy link
Collaborator

Adds most of the recommendations from @jridgewell 's very helpful stage 2 review comments.

Not included:

Add with to @@unscopeables

Note added in previous PR #76

Why are the with methods throwing a TypeError instead of clamping?

Issue opened #75

I don't think you need to include TypedArraySortCompare, I'm just gonna assume it's the same as the one in ecma262.

Will address as part of #51

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.

toSpliced changes in my polyfill pass tests, with my suggestion

spec.html Outdated Show resolved Hide resolved
spec.html Show resolved Hide resolved
Co-authored-by: Jordan Harband <ljharb@gmail.com>
1. Perform ! CreateDataPropertyOrThrow(_A_, _Pk_, _kValue_).
1. Set _k_ to _k_ + 1.
1. Let _i_ be 0.
1. Let _r_ be _actualStart_ + _actualDeleteCount_.
Copy link
Member

Choose a reason for hiding this comment

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

Just curious, what does _r_ stand for?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Was taken from #74 (comment)

The idea is that _i_ is index for insertions and _r_ is the index for reads. I was thinking of giving the variables more descriptive names to make this clearer but kept changing my mind back and forth.

@acutmore
Copy link
Collaborator Author

Thanks all!

@acutmore acutmore merged commit 61223a2 into tc39:main Feb 22, 2022
@acutmore acutmore deleted the post-review-improvments branch February 22, 2022 20:25
ljharb added a commit to es-shims/Array.prototype.toSpliced that referenced this pull request Mar 30, 2022
ljharb added a commit to es-shims/Array.prototype.toSpliced that referenced this pull request Mar 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants