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

fix: handle very tall and very wide tables #21

Merged
merged 7 commits into from
Oct 17, 2024
Merged

Conversation

mdonnalley
Copy link
Contributor

  • Handle very tall tables (greater than 50k) by chunking the rendering into pieces
  • Improve width adjustment by ensuring that all columns are adjusted if necessary

@W-16946299@
@W-16946304@

@iowillhoit
Copy link
Contributor

iowillhoit commented Oct 16, 2024

QA NOTES

  • 🟢 Error is thrown on duplicate columns
  • 🔴 iterm2 is flashing on the very-tall example:
Screen.Recording.2024-10-16.at.3.57.24.PM.mov
  • 🟢 terminal does not flash (and finishes much faster)
    • terminal finishes 100,000 rows in 32s
    • iterm2 finishes 100,000 rows in 1m28s
  • 🔴 very-tall example never exits and fails with a JavaScript heap out of memory error
    • iterm2 and terminal
  • 🟡 Seeing a strange render issue at the end of very-tall:
Screenshot 2024-10-16 at 4 00 48 PM
  • 🔴 In iterm2 the screen is clearing an I cannot scroll up beyond ~60 rows
    • This only happens when the output is chunked, reducing very-tall to 10_000 works fine
    • This is not happening in Terminal

@iowillhoit
Copy link
Contributor

QA UPDATE

  • 🟢 iterm2 no longer flashing
  • 🟢 Exiting properly (no heap error)
  • 🟡 Huh, now Terminal is taking much longer ~1m12s 🤪
  • 🟢 Strange render is gone
  • 🟢 Can scroll up in iterm2 now
  • 🟢 Error is shown if printTables([{...}, {...}]) has over 50,000 records
  • 🔴 printTables runs out of memory with 2 tables with 49_999
    • Not exactly sure where the threshold is... On my machine 15,000 worked and 24,000 failed.
import {printTables} from '../src/index.js'

const height = 49_999
const data = Array.from({length: height}, (_, i) => ({age: i, name: `Foo ${i}`}))

printTables([{
  columns: ['name', 'age'],
  data,
  headerOptions: {
    formatter: 'capitalCase',
  },
  horizontalAlignment: 'center',
  title: 'Very Tall 1',
  titleOptions: {bold: true},
},
{
  columns: ['name', 'age'],
  data,
  headerOptions: {
    formatter: 'capitalCase',
  },
  horizontalAlignment: 'center',
  title: 'Very Tall 2',
  titleOptions: {bold: true},
}])

@iowillhoit
Copy link
Contributor

QA UPDATE

  • 🟢 Error shows when printTables is over 30k
  • 🟢 2 tables render with 14,999 each
  • 🟢 4 tables render with 7,500
  • 🟢 Tables render side by side using maxWidth
  • 🟢 Looks good!

@iowillhoit iowillhoit merged commit 16ea9d8 into main Oct 17, 2024
11 checks passed
@iowillhoit iowillhoit deleted the mdonnalley/size-issues branch October 17, 2024 15:16
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.

2 participants