Skip to content

Add 2023 array methods to target features #52242

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

Merged
merged 8 commits into from
Jan 25, 2023

Conversation

OliverRadini
Copy link
Contributor

Fixes #52220

Adds es2023 array methods to target features.

I haven't added any unit tests for this - I couldn't find anything that tests this already, and I'm afraid I don't know enough about the repo to determine where these would make sense. Any pointers would be much appreciated :)

@typescript-bot typescript-bot added the For Milestone Bug PRs that fix a bug with a specific milestone label Jan 14, 2023
@Tobias-Scholz
Copy link
Contributor

Tobias-Scholz commented Jan 15, 2023

@OliverRadini - Take a look at indexAt.ts. You would need to write a similar file for findLastIndex and findLast.

@OliverRadini
Copy link
Contributor Author

@tobeyyyyy brilliant, thanks - I'll take a look and add that in

@OliverRadini
Copy link
Contributor Author

@tobeyyyyy Just taken a look, slightly confused though as it seems as though it may already be covered by the tests in findLast.ts. I'm probably getting things confused but I'm not sure what I'd add additionally to that.

@Tobias-Scholz
Copy link
Contributor

@OliverRadini - Ah, was not aware that a file like that already exists. The first line of the file defines all the targets for which baselines will be created. Right now, the only target in findLast.ts is esnext. You could now add es2022 to this list. Running the tests would generate a baseline like indexAt(target=es2021).errors.txt which includes the error message.

@OliverRadini
Copy link
Contributor Author

OliverRadini commented Jan 16, 2023

@tobeyyyyy ah brilliant, thanks. I amended the line as suggested and indeed did see the errors as expected. Is this the normal behaviour for the tests? I added the line, but now there are failing tests. I suppose I don't quite yet understand the workflow. The errors I get are:

tests/cases/compiler/findLast.ts(1,44): error TS2550: Property 'findLast' does not exist on type 'number[]'. Do you need to change your target library? Try changing the 'lib' compiler option to 'es2023' or later.
tests/cases/compiler/findLast.ts(2,51): error TS2550: Property 'findLast' does not exist on type 'string[]'. Do you need to change your target library? Try changing the 'lib' compiler option to 'es2023' or later.
tests/cases/compiler/findLast.ts(3,17): error TS2550: Property 'findLast' does not exist on type 'Int8Array'. Do you need to change your target library? Try changing the 'lib' compiler option to 'es2023' or later.

etc.

So, what I would expect to see after the change I added on a version which doesn't yet have findLast. Is this the expected behaviour?

@Jack-Works
Copy link
Contributor

Run > npx hereby baseline-accept

@OliverRadini OliverRadini marked this pull request as ready for review January 18, 2023 00:11
@OliverRadini
Copy link
Contributor Author

@microsoft-github-policy-service agree

@OliverRadini
Copy link
Contributor Author

@Jack-Works Great, thanks - ran that and all good now. Thanks for the help

@OliverRadini
Copy link
Contributor Author

Hopefully this is in a better state now, once I figured out a bit more about how the baseline stuff works 😂

@DanielRosenwasser DanielRosenwasser merged commit a311e25 into microsoft:main Jan 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide lib/target suggestions for findLast/findLastIndex
7 participants