Repository navigation
fix(upload): eliminate CSV upload freeze, per-cell JSON.parse storm, and large-output hang - #220
Merged
Conversation
…and large-output hang - Guard tryParseJsonLiteral by first/last character before calling JSON.parse so non-JSON cells never throw SyntaxError exceptions (70x faster applySmartValues on 60k-row CSVs) - Replace zip.apply(this, a) in csv2json transpose patch with an iterative loop so >125k rows never overflow the V8 call stack - Bypass typing debounce on discrete file uploads/encoding changes and fire tracker.discrete from computed conversion state so convertText runs once per upload - Slice only the first CSV record in InputPane before detectIndexedHeaderGroups so header detection never parses all rows - Render "Converting…" instead of "Resolve the error to see output." when conversion is pending with error === null - Skip heavy CodeMirror Lezer syntax/fold/bracket extensions above 512 KB and cap read-only JSON editor preview above 2 MB with a notice banner while preserving full output in Copy/Download - Add ErrorBoundary around converter panes and auto-detect UTF-16LE/BE BOMs in decodeUpload - Add bundled changelog entry #13 and rebuild app/dist Co-authored-by: David Boskovic <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Human author: David Boskovic ([email protected])
Why
Uploading a multi-megabyte CSV file (e.g. 60k–250k rows) froze the browser tab across two consecutive synchronous waves (
7.24 sat 60k rows;15.5 sat 120k rows;>39 sat 250k rows), flashed a false"Resolve the error to see output."message while conversion was in flight (error === null), and risked crashing the renderer into a blank page when mounting CodeMirror's Lezer syntax tree over 15–65 MB pretty-printed JSON or whentranspose: trueoverflowed the V8 call stack above 125k rows.What
applySmartValues(app/src/lib/convert.ts): GuardtryParseJsonLiteralby trimmed first/last characters (true,false,null,{...},[...],"...") before invokingJSON.parse, eliminating ~600,000 thrownSyntaxErrorexceptions on a 60k-row × 10-col CSV (2,188 ms→~23 ms).transpose(app/scripts/patch-csv2json.mjs): Replacezip.apply(this, a)incsvjson-csv2jsonwith an iterative row-by-column loop sotranspose: trueon>125,000rows never throwsRangeError: Maximum call stack size exceeded.app/src/App.tsx,app/src/components/InputPane.tsx): Bypass the typing debounce (150 ms/1,000 ms) whenreadFileorhandleUploadEncodingChangesetsinput, and firetracker.discretefrom an effect after the conversion result is computed soconvertTextruns once per upload instead of twice. InInputPane, slice only the first CSV record beforedetectIndexedHeaderGroupsso row-0 header inspection never parses all 60k+ rows.app/src/components/OutputPane.tsx): RenderConverting…(data-testid="output-converting-placeholder") when!inputEmpty && !outputText && !error, reserving"Resolve the error to see output."strictly forBoolean(error).app/src/components/JsonCodeMirror.tsx): Skip heavy Lezer syntax/fold/bracket extensions aboveLARGE_DOC_MAX_CHARS = 512 * 1024, and cap read-only JSON editor previews aboveREADONLY_PREVIEW_MAX_CHARS = 2_000_000to the first 512 KB (aligned to a line boundary) withdata-testid="large-output-preview-banner"whileCopyandDownloadcontinue to export the full output.ErrorBoundary& UTF-16LE/BE BOM auto-decode (app/src/components/ErrorBoundary.tsx,app/src/lib/files.ts,app/src/changelog/entries.ts): Wrap both converter panes in a recoverableErrorBoundary, sniff0xFF 0xFE/0xFE 0xFFBOMs indecodeUploadwhenencoding === "utf-8", add changelog entry#13, and rebuildapp/dist.Tradeoffs
CopyandDownload.How to Review
app/src/lib/convert.ts&app/scripts/patch-csv2json.mjs—tryParseJsonLiteralfast-path guard and iterativetransposeRowspatchapp/src/App.tsx&app/src/components/InputPane.tsx— immediatedebouncedInputsync on upload, post-conversiontracker.discrete, andsliceFirstCsvRecordapp/src/components/OutputPane.tsx&app/src/components/JsonCodeMirror.tsx—output-converting-placeholderandLARGE_DOC_MAX_CHARS/READONLY_PREVIEW_MAX_CHARSguardsapp/src/components/ErrorBoundary.tsx&app/src/lib/files.ts— recoverable pane error boundary and UTF-16 BOM sniffingapp/src/lib/__tests__/convert.test.ts,app/src/lib/__tests__/files.test.ts,app/src/App.test.tsx— unit and component regression testsVerification
cd app && npm test(30 test suites, 511 tests passing)cd app && npm run lint && npx tsc -b --noEmit && npm run buildbash .github/scripts/verify-seo.shagent-browser) benchmark onhttp://127.0.0.1:4173: uploading a 60,000-row × 10-column CSV (6.29 MB→15.75 MBJSON) completed in a single pass (979 mstotal long-task time vs.7,240 mspre-fix) withsawFalseError: falseand renderedlarge-output-preview-banner.🔗 Obvious Project · 🧵 Obvious Thread · 🔀 Obvious PR Artifact