Add Array.prototype.findLast() and Array.prototype.findLastIndex() #49636

m-basov · 2022-06-22T14:21:00Z

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

Testing performed
Added findLast.ts test file where 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 using this PR #46291 as an example.

Fixes #48829

Credits to Bloomberg colleagues who helped with reviews @acutmore @dragomirtitian @mkubilayk.

typescript-bot · 2022-06-22T14:21:06Z

Thanks for the PR! It looks like you've changed the TSServer protocol in some way. Please ensure that any changes here don't break consumers of the current TSServer API. For some extra review, we'll ping @sheetalkamat, @amcasey, @mjbvz, @minestarks for you. Feel free to loop in other consumers/maintainers if necessary

ghost · 2022-06-22T14:21:14Z

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

❌ m-basov sign now
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

mkubilayk · 2022-06-22T15:53:00Z

@DanielRosenwasser Has there been any changes to the CLA bot? I believe it used to accept contributions from the Bloomberg fork automatically.

Edit: Fixed now - thanks!

sandersn

Looks good except that TransformFlags ran out of space. @rbuckton, do you have any ideas for how to move any other members out of it?

sandersn · 2022-07-06T22:48:12Z

src/compiler/types.ts

+        ContainsES2016 = 1 << 10,
+        ContainsES2015 = 1 << 11,
+        ContainsGenerator = 1 << 12,
+        ContainsDestructuringAssignment = 1 << 13,


this now overlaps with ContainsTypeScriptClassSyntax = 1 << 13, which means we're officially out of space in TransformFlags. =(

Paging @rbuckton for advice.

Thanks for the review! I've assumed the overlap was intentional from the ES2022 PR but apparently it was fixed later by shifting Markers. There is no more space for that now though.

src/compiler/types.ts

rbuckton · 2022-08-02T18:08:34Z

tests/cases/compiler/findLast.ts

@@ -0,0 +1,27 @@
+// @target: es2023, esnext
+
+[0].findLast((item) => item === 0);


Can you add a test for the generic overload of findLast?

rbuckton · 2022-08-02T18:10:55Z

src/compiler/types.ts

@@ -6523,6 +6523,7 @@ namespace ts {
        ES2020 = 7,
        ES2021 = 8,
        ES2022 = 9,
+        ES2023 = 10,


@DanielRosenwasser: Do we normally add a new script target before that edition is ratified, or do we put everything in esnext until there's an official spec?

I think it's okay to put it into 2023 since we're expecting ratification before the stable release of TS5.0.

DanielRosenwasser · 2022-08-02T18:14:02Z

I believe we wait for ratification because the target isn't stable. lib might be different though?

sandersn · 2022-08-02T18:22:05Z

I'd vote to wait to add es2023 until that edition is ratified. It could go into esnext for now.

m-basov · 2022-08-25T13:14:15Z

I removed ES2023 target and left ES2023 lib as @DanielRosenwasser suggested. Could you take one more look @sandersn @rbuckton please? Thanks!

sandersn

Looks good. Can you merge from main so before I merge it?

m-basov · 2022-08-31T14:21:24Z

@sandersn done! Rebased on main.

src/lib/es2023.array.d.ts

graphemecluster · 2022-09-27T11:38:45Z

(I am not a member of TypeScript; I just reviewed based on my experience. Do point out me if I made something wrong.)

robpalme · 2022-10-07T10:29:56Z

Current status: @m-basov is waiting on #50450 by @graphemecluster to be merged. Once that lands he will rebase this PR.

graphemecluster · 2022-10-11T15:13:36Z

@robpalme Thanks, I've just resolved the conflicts of my PRs.
And sorry if I interpret this incorrectly, but if Bloomberg would like to see my PRs being merged, please do anything that speeds up the review process, or otherwise anything that could get it started quicker, as it has been a while since I work on them. Appreciate your support on it!

DanielRosenwasser · 2022-11-10T20:04:59Z

Can we just implement this based on the existing signatures of find and findIndex, and then handle the changes to findLast* in #50450? That one is a much harder change to evaluate w.r.t. ecosystem impact, and there's no reason to block this on it.

graphemecluster · 2022-11-10T23:28:21Z

@DanielRosenwasser I suppose the only difference is whether or not the this in the predicate should be typed void (and whether to repeat the documentation). The others are, most likely, errors not existing in the current find* signature.

m-basov · 2022-11-11T10:08:52Z

Sure, I will rebase my changes on the latest main branch.

UPD. The branch is rebased now but CLA isn't working for some reason.

DanielRosenwasser · 2023-01-13T01:13:37Z

Looks like we have some merge conflicts. Would you be able to fix those up?

Also, @rbuckton would you be able to take a second look?

DanielRosenwasser · 2023-01-13T01:14:19Z

@typescript-bot pack this
@typescript-bot test top100
@typescript-bot run dt

typescript-bot · 2023-01-13T01:14:21Z

Heya @DanielRosenwasser, I've started to run the tarball bundle task on this PR at 95fab91. You can monitor the build here.

typescript-bot · 2023-01-13T01:14:21Z

Heya @DanielRosenwasser, I've started to run the diff-based top-repos suite on this PR at 95fab91. You can monitor the build here.

Update: The results are in!

typescript-bot · 2023-01-13T01:14:21Z

Heya @DanielRosenwasser, I've started to run the parallelized Definitely Typed test suite on this PR at 95fab91. You can monitor the build here.

typescript-bot · 2023-01-13T01:16:39Z

Hey @DanielRosenwasser, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/142507/artifacts?artifactName=tgz&fileId=7EC6C5954393EDF3948E05E7C7D7AA40EB6F5EEDEA27C4226952B5F1C991319F02&fileName=/typescript-5.0.0-insiders.20230113.tgz"
    }
}

and then running npm install.

There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@5.0.0-pr-49636-19".;

typescript-bot · 2023-01-13T02:56:14Z

@DanielRosenwasser Here are the results of running the top-repos suite comparing main and refs/pull/49636/merge:

Everything looks good!

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

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

m-basov · 2023-01-13T09:39:45Z

@DanielRosenwasser thanks for review. I've fixed the merge conflicts.

@graphemecluster thanks for spotting the issue with Uint8ClampedArray[]. It's fixed now but I cannot find your comment in the GH UI (I noticed it on email).

m-basov · 2023-01-13T10:58:15Z

@microsoft-github-policy-service agree company="Bloomberg"

rbuckton

I'm a bit wary about adding an es2023 lib given that the committee has not yet ratified ES2023, but given that Array find from last is at Stage 4, this is probably fine.

typescript-bot assigned sandersn Jun 22, 2022

typescript-bot added the For Milestone Bug PRs that fix a bug with a specific milestone label Jun 22, 2022

mkubilayk mentioned this pull request Jun 24, 2022

Add case to capture enum subtype reduction (addresses #35867) #49675

Merged

m-basov closed this Jul 1, 2022

m-basov reopened this Jul 1, 2022

sandersn requested changes Jul 6, 2022

View reviewed changes

rbuckton requested changes Aug 2, 2022

View reviewed changes

m-basov mentioned this pull request Aug 25, 2022

Remove ES2023 target from Array.findLast() PR bloomberg/TypeScript#75

Merged

m-basov requested review from sandersn and rbuckton and removed request for sandersn and rbuckton August 25, 2022 13:12

sandersn approved these changes Aug 30, 2022

View reviewed changes

m-basov force-pushed the oss-hackathon-48829 branch from 5b524d5 to c7518db Compare August 31, 2022 13:55

graphemecluster reviewed Sep 27, 2022

View reviewed changes

src/lib/es2023.array.d.ts Outdated Show resolved Hide resolved

src/lib/es2023.array.d.ts Outdated Show resolved Hide resolved

src/lib/es2023.array.d.ts Outdated Show resolved Hide resolved

src/lib/es2023.array.d.ts Outdated Show resolved Hide resolved

m-basov force-pushed the oss-hackathon-48829 branch from c7518db to a6a6e74 Compare December 16, 2022 13:08

m-basov requested a review from rbuckton December 16, 2022 14:11

m-basov force-pushed the oss-hackathon-48829 branch from 4cd2162 to 95fab91 Compare December 16, 2022 20:47

DanielRosenwasser approved these changes Jan 13, 2023

View reviewed changes

DanielRosenwasser mentioned this pull request Jan 13, 2023

Provide lib/target suggestions for findLast/findLastIndex #52220

Closed

m-basov and others added 4 commits January 13, 2023 09:10

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

044822b

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

Remove this: void

175c012

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

Fix array brackets

a4b18fa

Fix baselines after rebase and Uint8ClampedArray fix

9bf9ee1

m-basov force-pushed the oss-hackathon-48829 branch from 95fab91 to 9bf9ee1 Compare January 13, 2023 09:27

rbuckton approved these changes Jan 13, 2023

View reviewed changes

sandersn merged commit cb4c768 into microsoft:main Jan 13, 2023

thernstig mentioned this pull request Nov 3, 2023

ES2023 missing for target in docs? #56303

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Array.prototype.findLast() and Array.prototype.findLastIndex() #49636

Add Array.prototype.findLast() and Array.prototype.findLastIndex() #49636

m-basov commented Jun 22, 2022 •

edited

Loading

typescript-bot commented Jun 22, 2022

ghost commented Jun 22, 2022

mkubilayk commented Jun 22, 2022 •

edited

Loading

sandersn left a comment

sandersn Jul 6, 2022

m-basov Jul 7, 2022

rbuckton Aug 2, 2022

rbuckton Aug 2, 2022

DanielRosenwasser Jan 13, 2023 •

edited

Loading

DanielRosenwasser commented Aug 2, 2022 •

edited

Loading

sandersn commented Aug 2, 2022

m-basov commented Aug 25, 2022

sandersn left a comment

m-basov commented Aug 31, 2022

graphemecluster commented Sep 27, 2022

robpalme commented Oct 7, 2022

graphemecluster commented Oct 11, 2022

DanielRosenwasser commented Nov 10, 2022

graphemecluster commented Nov 10, 2022

m-basov commented Nov 11, 2022 •

edited

Loading

DanielRosenwasser commented Jan 13, 2023

DanielRosenwasser commented Jan 13, 2023

typescript-bot commented Jan 13, 2023 •

edited

Loading

typescript-bot commented Jan 13, 2023 •

edited

Loading

typescript-bot commented Jan 13, 2023 •

edited

Loading

typescript-bot commented Jan 13, 2023 •

edited

Loading

typescript-bot commented Jan 13, 2023

m-basov commented Jan 13, 2023

m-basov commented Jan 13, 2023

rbuckton left a comment

		@@ -0,0 +1,27 @@
		// @target: es2023, esnext

		[0].findLast((item) => item === 0);

Add Array.prototype.findLast() and Array.prototype.findLastIndex() #49636

Add Array.prototype.findLast() and Array.prototype.findLastIndex() #49636

Conversation

m-basov commented Jun 22, 2022 • edited Loading

typescript-bot commented Jun 22, 2022

ghost commented Jun 22, 2022

mkubilayk commented Jun 22, 2022 • edited Loading

sandersn left a comment

Choose a reason for hiding this comment

sandersn Jul 6, 2022

Choose a reason for hiding this comment

m-basov Jul 7, 2022

Choose a reason for hiding this comment

rbuckton Aug 2, 2022

Choose a reason for hiding this comment

rbuckton Aug 2, 2022

Choose a reason for hiding this comment

DanielRosenwasser Jan 13, 2023 • edited Loading

Choose a reason for hiding this comment

DanielRosenwasser commented Aug 2, 2022 • edited Loading

sandersn commented Aug 2, 2022

m-basov commented Aug 25, 2022

sandersn left a comment

Choose a reason for hiding this comment

m-basov commented Aug 31, 2022

graphemecluster commented Sep 27, 2022

robpalme commented Oct 7, 2022

graphemecluster commented Oct 11, 2022

DanielRosenwasser commented Nov 10, 2022

graphemecluster commented Nov 10, 2022

m-basov commented Nov 11, 2022 • edited Loading

DanielRosenwasser commented Jan 13, 2023

DanielRosenwasser commented Jan 13, 2023

typescript-bot commented Jan 13, 2023 • edited Loading

typescript-bot commented Jan 13, 2023 • edited Loading

typescript-bot commented Jan 13, 2023 • edited Loading

typescript-bot commented Jan 13, 2023 • edited Loading

typescript-bot commented Jan 13, 2023

m-basov commented Jan 13, 2023

m-basov commented Jan 13, 2023

rbuckton left a comment

Choose a reason for hiding this comment

m-basov commented Jun 22, 2022 •

edited

Loading

mkubilayk commented Jun 22, 2022 •

edited

Loading

DanielRosenwasser Jan 13, 2023 •

edited

Loading

DanielRosenwasser commented Aug 2, 2022 •

edited

Loading

m-basov commented Nov 11, 2022 •

edited

Loading

typescript-bot commented Jan 13, 2023 •

edited

Loading

typescript-bot commented Jan 13, 2023 •

edited

Loading

typescript-bot commented Jan 13, 2023 •

edited

Loading

typescript-bot commented Jan 13, 2023 •

edited

Loading