GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Accurate Array.prototype.flat definition by eilvelia · Pull Request #32131 · microsoft/TypeScript

Conversation

@eilvelia

@eilvelia eilvelia (eilvelia) commented Jun 27, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Fixes #29604 (and #29741).

Examples:

const a0 = [1, [2]].flat() // const a0: number[]
const a1 = [[1], 'a'].flat(1) // const a1: (string | number)[]
const n: number = 1
const a2 = [1, [2]].flat(n) // const a2: (number | number[])[]
const b0 = [[1], ['a']].flat(1) // const b0: (string | number)[]
const c0 = [
  [[1]],
  [[2]]
].flat(2) // const c0: number[]
const arr = [[[1, [2, [3, [4]]]]]]
const d0 = arr.flat(2) // const d0: (number | (number | (number | number[])[])[])[]
const d1 = arr.flat(n) // const d1: (number | number[] | (number | number[])[] | (number | (number | number[])[])[] | (number | (number | (number | number[])[])[])[] | (number | (number | (number | number[])[])[])[][])[]

The old declaration returns any[] for all cases except c0, d0 and supports depth only up to 7 (this one supports up to 20).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

goodmind, IllusionMH, tonivj5, phaux, Jaraujo6, and chocolateboy reacted with thumbs up emoji
All reactions
  • 👍 6 reactions
@eilvelia
Better Array.prototype.flat definition
ade67c6
@eilvelia eilvelia (eilvelia) mentioned this pull request Jun 28, 2019
feat: add flat/flatMap meta/flow#6948
Closed
@RyanCavanaugh

Ryan Cavanaugh (RyanCavanaugh) commented Jul 16, 2019

Copy link
Copy Markdown
Member

TypeScript Bot (@typescript-bot) test this
TypeScript Bot (@typescript-bot) run dt
TypeScript Bot (@typescript-bot) user test this

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@typescript-bot

TypeScript Bot (typescript-bot) commented Jul 16, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Heya Ryan Cavanaugh (@RyanCavanaugh), I've started to run the extended test suite on this PR at ade67c6. You can monitor the build here. It should now contribute to this PR's status checks.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@typescript-bot

TypeScript Bot (typescript-bot) commented Jul 16, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Heya Ryan Cavanaugh (@RyanCavanaugh), I've started to run the parallelized community code test suite on this PR at ade67c6. You can monitor the build here. It should now contribute to this PR's status checks.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@typescript-bot

TypeScript Bot (typescript-bot) commented Jul 16, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Heya Ryan Cavanaugh (@RyanCavanaugh), I've started to run the parallelized Definitely Typed test suite on this PR at ade67c6. You can monitor the build here. It should now contribute to this PR's status checks.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) self-assigned this Jul 16, 2019
@typescript-bot

TypeScript Bot (typescript-bot) commented Jul 16, 2019

Copy link
Copy Markdown
Contributor

The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@eilvelia

eilvelia (eilvelia) commented Jul 17, 2019

Copy link
Copy Markdown
Contributor Author

I'm not sure, should I merge it? The failed tests don't look related to Array#flat.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sandersn Nathan Shively-Sanders (sandersn) added the For Milestone Bug PRs that fix a bug with a specific milestone label Feb 1, 2020
sandersn
Nathan Shively-Sanders (sandersn) requested changes Feb 27, 2020
View reviewed changes

@sandersn Nathan Shively-Sanders (sandersn) left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I think this needs a synthetic performance test to figure out if the new code is faster or slower. Maybe we can guess just by thinking about the compiler implementation though.

A performance test would look like several thousand calls to flat from the mix of different types supported by the old version, eg ReadonlyArray<ReadonlyArray<U[]>[]>. Plus maybe longer versions of the same thing.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread src/lib/es2019.array.d.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/lib/es2019.array.d.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/lib/es2019.array.d.ts Outdated
@@ -1,3 +1,10 @@
type Flat<A, D extends number> = {

@sandersn Nathan Shively-Sanders (sandersn) Feb 27, 2020

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

we should check for existing types named Flat in some code base -- probably it's fine, but might not be.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@sandersn Nathan Shively-Sanders (sandersn) Feb 27, 2020

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Probably Definitely Typed is a big enough sample -- grepping for class Flat, interface Flat, type Flat should give you (almost?) all of the uses.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@sandersn Nathan Shively-Sanders (sandersn) Feb 27, 2020

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

  • seen has class Flat (appropriate for a 3D renderer)
  • array.prototype.flat has interface Flat (and FlatMap)

Seen is barely used, but array.prototype.flat usage is quite widespread.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
eilvelia (eilvelia) added 2 commits February 27, 2020 03:17
@eilvelia
Resolve merge conflicts
7b74bd8
@eilvelia
Use more meaningful names
1b31861
sandersn
Nathan Shively-Sanders (sandersn) requested changes Mar 11, 2020
View reviewed changes

@sandersn Nathan Shively-Sanders (sandersn) left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Also needs results from a synthetic performance test to see which is faster.

Data from parse/bind/check time of lib.d.ts (without --skipLibCheck) would be nice too, but not necessary. It would be hard to show a significant improvement there.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread src/lib/es2019.array.d.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

@sandersn

Nathan Shively-Sanders (sandersn) commented Mar 26, 2020

Copy link
Copy Markdown
Member

Bannerets do you have time for a performance test? Otherwise I'd like to close the PR to keep the number of open PRs manageable.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

eilvelia (eilvelia) added 2 commits March 27, 2020 01:19
@eilvelia
Rename 'Flat' to 'FlatArray'
c92d25f
@eilvelia
Merge branch 'master' into array-flat
a7404d1
@eilvelia

eilvelia (eilvelia) commented Mar 27, 2020

Copy link
Copy Markdown
Contributor Author

I'm not sure what's the correct way to write synthetic performance tests for types (since I don't know when the compiler can perform caching, etc.). Is there a utility in the typescript repository for this?

For example, I wrote this generator:

$ cat gen.js
for (let i = 0; i < 4000; i++) {
  console.log(
`declare var arr1_${i}: ReadonlyArray<ReadonlyArray<(string | number)[]>[]>;
arr1_${i}.flat(3);
declare var arr2_${i}: ReadonlyArray<ReadonlyArray<number>[]>;
arr2_${i}.flat(2);
declare var arr3_${i}: ReadonlyArray<string[][][][][][]>;
arr3_${i}.flat(4);
declare var arr4_${i}: ReadonlyArray<number[][]>;
arr4_${i}.flat(2);
declare var arr5_${i}: ReadonlyArray<ReadonlyArray<ReadonlyArray<string[]>>>;
arr5_${i}.flat(3);`)
}
$ mv built built-new
$ git checkout 7f1df6e53e
$ npm run build
$ mv built built-old
$ node gen.js > ttest.ts
$ time node ./built-new/local/tsc.js --noEmit --lib es2019.array ttest.ts
       61.21 real        63.06 user         0.54 sys
$ time node ./built-old/local/tsc.js --noEmit --lib es2019.array ttest.ts
      115.83 real       117.41 user         0.89 sys

It shows the unbelievable 88% increase in performance (I've tried to run it multiple times).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sandersn

Nathan Shively-Sanders (sandersn) commented Apr 7, 2020

Copy link
Copy Markdown
Member

I switched to object literal types instead of string/number because those are fresh on each usage. But with 4000 copies, master now runs out of memory! So I reduced the number to 2000 and observed a ratio that's even better than yours.

Before

real	0m19.253s
user	0m24.197s
sys	0m0.667s

After

real	0m9.005s
user	0m11.581s
sys	0m0.245s

gen.js

for (let i = 0; i < 4000; i++) {
  console.log(
`declare var arr1_${i}: ReadonlyArray<ReadonlyArray<{ a_${i}: number }[]>[]>;
arr1_${i}.flat(3);
declare var arr2_${i}: ReadonlyArray<ReadonlyArray<{ b_${i}: number }>[]>;
arr2_${i}.flat(2);
declare var arr3_${i}: ReadonlyArray<{ c_${i}: number }[][][][][][]>;
arr3_${i}.flat(4);
declare var arr4_${i}: ReadonlyArray<{ d_${i}: number }[][]>;
arr4_${i}.flat(2);
declare var arr5_${i}: ReadonlyArray<ReadonlyArray<ReadonlyArray<{ e_${i}: number }[]>>>;
arr5_${i}.flat(3);`)
}
eilvelia and ExE-Boss reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sandersn

Nathan Shively-Sanders (sandersn) commented Apr 7, 2020

Copy link
Copy Markdown
Member

The reason is probably that there are 8 overloads, with the shallower-array overloads near the bottom. Conditional type instantiation and overload resolution are both linear, but shallower arrays resolve first with the conditional type.

It could be also that conditional type instantiation is more efficient than overload resolution.

ExE-Boss reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

sandersn
Nathan Shively-Sanders (sandersn) approved these changes Apr 7, 2020
View reviewed changes

@sandersn Nathan Shively-Sanders (sandersn) left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Looks much more efficient in practise.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@sandersn

Nathan Shively-Sanders (sandersn) commented Apr 7, 2020

Copy link
Copy Markdown
Member

I re-ran with --diagnostics and noticed that subsequent runs of the old compiler are much faster. I averaged four runs (not because that's a magic number, but because that's how much patience I had) and the average is 9.15 vs 9.12. So I think actually the two are about the same.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sandersn
Nathan Shively-Sanders (sandersn) merged commit c47aca0 into microsoft:master Apr 7, 2020
@jcalz

Joe Calzaretta (jcalz) commented May 1, 2020

Copy link
Copy Markdown
Contributor

I find it interesting that these typings

type FlatArray<Arr, Depth extends number> = {
    "done": Arr,
    "recur": Arr extends ReadonlyArray<infer InnerArr>
        ? FlatArray<InnerArr, [-1, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20][Depth]>
        : Arr
}[Depth extends -1 ? "done" : "recur"];

use the recursive conditional type "trick" that circumvents the restrictions discussed in #26980 and was explicitly warned against by @ahejlsberg and similarly not given the seal of approval by @sandersn. I've used the sort of depth limiting being done here, but always with the understanding that this is murky territory where dragons and other things that go bump in the night reside, and never with the thought that such code would make it into anything standard.

So, uh, what's the story here? Should I turn in my badge and service weapon? Should internal affairs be brought in? Does anything make sense anymore? Thanks!

smoogly, rhermes62, Ciantic, dead-claudia, levenleven, ExE-Boss, and InExtremaRes reacted with thumbs up emoji millsp, dead-claudia, eilvelia, yaroslavvoloshchuk-codaio, ExE-Boss, and InExtremaRes reacted with laugh emoji yvele reacted with eyes emoji
All reactions
  • 👍 7 reactions
  • 😄 6 reactions
  • 👀 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@jcalz Joe Calzaretta (jcalz) mentioned this pull request May 1, 2020
Feature request: lift the circular constraint for conditional types #26980
Closed
4 tasks done
@dead-claudia

Claudia Meadows (dead-claudia) commented May 1, 2020

Copy link
Copy Markdown

Joe Calzaretta (@jcalz) Ironically, this PR was authored by Nathan Shively-Sanders (@sandersn)... 😂

ExE-Boss reacted with thumbs up emoji ExE-Boss reacted with laugh emoji
All reactions
  • 👍 1 reaction
  • 😄 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@0kku Okku (0kku) mentioned this pull request May 2, 2020
RangeError when trying to infer type for Array.prototype.flat() #38298
Closed
@GuiltyDolphin Ben Moon (GuiltyDolphin) mentioned this pull request Jun 3, 2021
Trying to flatten an array with unknown depth causes a type error #44408
Open
@PKrasinski Przemysław Krasiński (PKrasinski) mentioned this pull request Sep 8, 2023
Unlimited Array.prototype.flat depth #55686
Closed
@microsoft Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@sandersn Nathan Shively-Sanders (sandersn) sandersn approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh)

Labels

For Milestone Bug PRs that fix a bug with a specific milestone

Projects

PR Backlog
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Wrongly infers any[] from Array.prototype.flat, Array.prototype.concat

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@eilvelia @RyanCavanaugh @typescript-bot @sandersn @jcalz @dead-claudia
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.