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

Use Array.flat instead of array-flatten #5677

Merged
merged 1 commit into from
Aug 1, 2024

Conversation

almic
Copy link
Contributor

@almic almic commented May 20, 2024

Depends upon #5595, Adopt Node@18 as minimum supported version.

With this new version, we get Array.flat(depth), which can be used instead of a package. Infinity would have the same behavior as the package, recursively unwrapping nested arrays indefinitely. But, perhaps it might be worth selecting a more reasonable value, to avoid a RangeError, such as when an array contains a reference to itself.

Note

Mimimum Node.js version supported is 11

@wesleytodd
Copy link
Member

This is going to fail CI for now, but I figured it was better to approve the CI run for reference. I think this is a good change, but we have a bunch on our plate first so please be patient with us as we get the other things done first.

@admsev

This comment was marked as spam.

@gooroodev

This comment was marked as spam.

@mmaazahmed

This comment was marked as off-topic.

@krzysdz

This comment was marked as off-topic.

@mmaazahmed

This comment was marked as off-topic.

@wesleytodd

This comment was marked as off-topic.

@jonchurch jonchurch requested a review from a team June 10, 2024 21:27
@wesleytodd wesleytodd changed the base branch from 5.x to 5.0 July 25, 2024 21:53
Copy link
Member

@UlisesGascon UlisesGascon left a comment

Choose a reason for hiding this comment

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

This now is unblocked as #5490 was merged 👍

Copy link
Member

@sheplu sheplu left a comment

Choose a reason for hiding this comment

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

LGTM but a rebase is needed first

@jonchurch jonchurch closed this Aug 1, 2024
@jonchurch jonchurch reopened this Aug 1, 2024
@jonchurch
Copy link
Member

clopened to rerun CI but forgot 5.0 hasn't dropped the old versions in CI

Copy link
Member

@jonchurch jonchurch left a comment

Choose a reason for hiding this comment

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

This is fine to merge, we know it breaks CI on the node versions v5 doesn't support

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants