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

Add .findLast() and .findLastIndex() #69

Merged
merged 2 commits into from
Jun 22, 2022

Conversation

m-basov
Copy link

@m-basov m-basov commented Jun 16, 2022

microsoft#48829

Describe your changes
Add type definitions for Array.prototype.findLast() and Array.prototype.findLastIndex() methods. Both methods were added to the es2023 target but the target didn't exist so I've created it. The definitions are identical to .find() and .findIndex() but with the updated comments.

Testing performed
Added findLast.ts test file were all Array interfaces try to call .findLast() and .findLastIndex().
Also, tested it manually by running:

  • tsc with the es5 target and libs: []
  • tsc with the es5 target and libs: ["es2023"]
  • tsc with the es5 target and libs: ["esnext"]

Additional context
The es2023 target were added with the help of this PR microsoft#46291

Signed-off-by: mbasov2 <mbasov2@bloomberg.net>
Signed-off-by: mbasov2 <mbasov2@bloomberg.net>
Copy link

@mkubilayk mkubilayk 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 - one small comment.

@@ -0,0 +1,18 @@
//// [callChainWithSuper.ts]

Choose a reason for hiding this comment

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

Is this test relevant to your changes?

Copy link
Author

Choose a reason for hiding this comment

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

I believe it was created because the es2023 target was added (filename is callChainWithSuper(target=es2023).js). This file was added for the es2022 target as well.

@acutmore
Copy link

I wonder if adding es2023 as a target should be separated out as a separate PR. es2023 doesn't exist yet as a spec so it doesn't feel right that TypeScript would offer it as a target yet. 'esnext' seems more appropriate for now.

@mkubilayk
Copy link

Daniel suggests adding them to ES2023 in microsoft#48829 (comment).

@acutmore
Copy link

Daniel suggests adding them to ES2023 in microsoft#48829 (comment).

Ah cool. Thanks. I can also see that TypeScript also added es2022 before the spec was official so does seem like there is precedent for this. I'm happy.

@mkubilayk mkubilayk merged commit 3b610c3 into bloomberg:oss-hackathon-48829 Jun 22, 2022
m-basov added a commit that referenced this pull request Aug 31, 2022
* Add .findLast(), .findLastIndex(), and es2023 target

Signed-off-by: mbasov2 <mbasov2@bloomberg.net>

* Update baselines

Signed-off-by: mbasov2 <mbasov2@bloomberg.net>
m-basov added a commit that referenced this pull request Dec 16, 2022
* Add .findLast(), .findLastIndex(), and es2023 target

Signed-off-by: mbasov2 <mbasov2@bloomberg.net>

* Update baselines

Signed-off-by: mbasov2 <mbasov2@bloomberg.net>

Remove ES2023 target (#75)

Signed-off-by: mbasov2 <mbasov2@bloomberg.net>
m-basov added a commit that referenced this pull request Jan 13, 2023
* Add .findLast(), .findLastIndex(), and es2023 target

Signed-off-by: mbasov2 <mbasov2@bloomberg.net>

* Update baselines

Signed-off-by: mbasov2 <mbasov2@bloomberg.net>

Remove ES2023 target (#75)

Signed-off-by: mbasov2 <mbasov2@bloomberg.net>
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

Successfully merging this pull request may close these issues.

4 participants