@uppy/core: improve performance of validating & uploading files #4402

mifi · 2023-04-05T16:07:46Z

Pull out totals validation for Restrictions (maxTotalFileSize, maxNumberOfFiles)

don't do it for every file added, as it's very slow
instead do the check at the end, after all files are added/validated individually.
this allows us to easily work with 10k+ files
fixes #4389

also:

refactor/reduce code duplication between addFile/addFiles Reduce code duplication #4377
refactor/fix lint for Dashboard props

remake of #4388

don't do it for every file added, as it's very slow instead do the check at the end when all files are added. this allows us to easily work with 10k+ files fixes #4389

packages/@uppy/core/src/Uppy.js

Murderlon

Very nice!

packages/@uppy/core/src/Restricter.js

packages/@uppy/core/src/Uppy.js

packages/@uppy/dashboard/src/components/FileList.jsx

Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>

instead make more specific methods for sub-validation also rename validateTotals to validateAggregateRestrictions

- handle errors centrally so that we can limit the amount of toasts (informers) sent to the users (prevent flooding hundreds/thousands of them) - introduce FileRestrictionError which is a restriction error for a specific file - introduce isUserFacing field for RestrictionError

mifi · 2023-04-06T11:13:26Z

I noticed another problem with massive amounts of toasts being generated if per file restrictions are added, so I did some rewriting so we can handle showing of toasts more centrally:

handle errors centrally so that we can limit the amount of toasts (informers) sent to the users (prevent flooding hundreds/thousands of them)
introduce FileRestrictionError which is a restriction error for a specific file
introduce isUserFacing field for RestrictionError which defines whether we should show it to the user or not (some restrictions are not to be shown to users)

packages/@uppy/core/src/Uppy.js

- show "%{count} additional restrictions were not fulfilled" for any restriction errors more than 4 - refactor/rename methods - improve ghost logic/comments

- introduce new event "upload-start" that can contain multiple files - make a new patchFilesState method to allow updating more files - unify "upload-start" logic in all plugins (send it before files start uploading) - defer slicing buffer until we need the data - refactor to reuse code

maybe it fixes the error

This reverts commit ff3e580.

socket-security · 2023-04-10T06:36:31Z

New dependency changes detected. Learn more about Socket for GitHub ↗︎

👍 No new dependency issues detected in pull request

Bot Commands

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore foo@1.0.0 bar@* or ignore all packages with @SocketSecurity ignore-all

Pull request alert summary

Issue	Status
Install scripts	✅ 0 issues
Native code	✅ 0 issues
Bin script shell injection	✅ 0 issues
Unresolved require	✅ 0 issues
Invalid package.json	✅ 0 issues
HTTP dependency	✅ 0 issues
Git dependency	✅ 0 issues
Potential typo squat	✅ 0 issues
Known Malware	✅ 0 issues
Telemetry	✅ 0 issues
Protestware/Troll package	✅ 0 issues

📊 Modified Dependency Overview:

➕ Added Package	Capability Access	`+/-` Transitive Count	Publisher
parcel@2.0.0-nightly.1278	None	`+0`	devongovett

⬆️ Updated Package	Version Diff	Added Capability Access	`+/-` Transitive Count	Publisher
cypress@12.9.0	10.11.0...12.9.0	environment	`+0/-0`	atofstryker

🚮 Removed packages: @parcel/transformer-vue@2.7.0

mifi · 2023-04-10T08:08:40Z

e2e issue seems to be cause by parcel: parcel-bundler/parcel#4155

however they haven't yet released a new version

mifi · 2023-04-10T08:39:29Z

ok i fixed it by upgrading to latest nightly build. do we accept the security risk of cypress' install script?

Murderlon

Big PR with many improvements! I think this should be tested locally by at least another person before we merge.

packages/@uppy/provider-views/src/ProviderView/ProviderView.jsx

packages/@uppy/utils/src/fileFilters.js

packages/@uppy/core/src/Uppy.js

merge it with RestrictionError

looks like the e2e tests are doing its job 👏

mifi · 2023-04-13T12:38:18Z

anyone else wanna do some testing/review of the code first or shall we merge this?:)

arturi · 2023-04-13T12:52:47Z

Yes, will test today!

arturi · 2023-04-14T16:10:59Z

packages/@uppy/aws-s3-multipart/src/MultipartUploader.js

-        this.#chunks[j++] = chunk
+
+        // Defer data fetching/slicing until we actually need the data, because it's slow if we have a lot of files
+        const getData = () => {


We need to remember to port these to #4205, right?

yes, seems like that pr will get a merge conflict once we merge this

arturi · 2023-04-14T16:58:33Z

Tested, 10,000 small files — quick and smooth, tested S3 Multipart upload for good measure too — works :) Canceling the upload with 10,000 files results in UI not reacting for a couple of seconds (not frozen, but no feedback), then cancellation is successful. Not sure if we could (later) look into setting files: {} in state right away for better UX.

mifi · 2023-04-15T15:11:33Z

I noticed that too. Maybe we should introduce a "loading" state in Uppy when we need to wait for something, where we show a full-screen loading that blocks any user interaction. Else we could run into race conditions, e.g. if the user tries to cancel again, or they try to upload again, but the previous upload is still going on in the background, possibly causing issues.

| Package | Version | Package | Version | | ------------------------- | ------- | ------------------------- | ------- | | @uppy/angular | 0.5.2 | @uppy/progress-bar | 3.0.2 | | @uppy/audio | 1.1.1 | @uppy/provider-views | 3.3.0 | | @uppy/aws-s3 | 3.1.0 | @uppy/react | 3.1.2 | | @uppy/aws-s3-multipart | 3.2.0 | @uppy/react-native | 0.5.1 | | @uppy/box | 2.1.1 | @uppy/redux-dev-tools | 3.0.2 | | @uppy/companion | 4.5.0 | @uppy/remote-sources | 1.0.3 | | @uppy/companion-client | 3.1.3 | @uppy/screen-capture | 3.1.1 | | @uppy/compressor | 1.0.2 | @uppy/status-bar | 3.1.1 | | @uppy/core | 3.2.0 | @uppy/store-default | 3.0.3 | | @uppy/dashboard | 3.4.0 | @uppy/store-redux | 3.0.3 | | @uppy/drag-drop | 3.0.2 | @uppy/svelte | 3.0.2 | | @uppy/dropbox | 3.1.1 | @uppy/thumbnail-generator | 3.0.3 | | @uppy/facebook | 3.1.1 | @uppy/transloadit | 3.1.3 | | @uppy/file-input | 3.0.2 | @uppy/tus | 3.1.0 | | @uppy/form | 3.0.2 | @uppy/unsplash | 3.2.1 | | @uppy/golden-retriever | 3.0.3 | @uppy/url | 3.3.1 | | @uppy/google-drive | 3.1.1 | @uppy/utils | 5.3.0 | | @uppy/image-editor | 2.1.2 | @uppy/vue | 1.0.2 | | @uppy/informer | 3.0.2 | @uppy/webcam | 3.3.1 | | @uppy/instagram | 3.1.1 | @uppy/xhr-upload | 3.2.0 | | @uppy/locales | 3.2.0 | @uppy/zoom | 2.1.1 | | @uppy/onedrive | 3.1.1 | uppy | 3.8.0 | - @uppy/companion: increase max limits for remote file list operations (Mikael Finstad / #4417) - @uppy/xhr-upload: fix type in README.md (Top Master / #4416) - @uppy/core: improve performance of validating & uploading files (Mikael Finstad / #4402) - @uppy/provider-views: Concurrent file listing (Mikael Finstad / #4401) - @uppy/core,@uppy/locales,@uppy/provider-views: User feedback adding recursive folders take 2 (Mikael Finstad / #4399) - @uppy/dashboard: Single File Mode: fix layout and make optional (Artur Paikin / #4374) - @uppy/informer: add a check in `TransitionGroup` when component is null (Juan Belej / #4410) - meta: Fix logos in all the readmes (Artur Paikin / #4407) - meta: fix logo in readme (Kid / #4403)

| Package | Version | Package | Version | | ------------------------- | ------- | ------------------------- | ------- | | @uppy/angular | 0.5.2 | @uppy/progress-bar | 3.0.2 | | @uppy/audio | 1.1.1 | @uppy/provider-views | 3.3.0 | | @uppy/aws-s3 | 3.1.0 | @uppy/react | 3.1.2 | | @uppy/aws-s3-multipart | 3.2.0 | @uppy/react-native | 0.5.1 | | @uppy/box | 2.1.1 | @uppy/redux-dev-tools | 3.0.2 | | @uppy/companion | 4.5.0 | @uppy/remote-sources | 1.0.3 | | @uppy/companion-client | 3.1.3 | @uppy/screen-capture | 3.1.1 | | @uppy/compressor | 1.0.2 | @uppy/status-bar | 3.1.1 | | @uppy/core | 3.2.0 | @uppy/store-default | 3.0.3 | | @uppy/dashboard | 3.4.0 | @uppy/store-redux | 3.0.3 | | @uppy/drag-drop | 3.0.2 | @uppy/svelte | 3.0.2 | | @uppy/dropbox | 3.1.1 | @uppy/thumbnail-generator | 3.0.3 | | @uppy/facebook | 3.1.1 | @uppy/transloadit | 3.1.3 | | @uppy/file-input | 3.0.2 | @uppy/tus | 3.1.0 | | @uppy/form | 3.0.2 | @uppy/unsplash | 3.2.1 | | @uppy/golden-retriever | 3.0.3 | @uppy/url | 3.3.1 | | @uppy/google-drive | 3.1.1 | @uppy/utils | 5.3.0 | | @uppy/image-editor | 2.1.2 | @uppy/vue | 1.0.2 | | @uppy/informer | 3.0.2 | @uppy/webcam | 3.3.1 | | @uppy/instagram | 3.1.1 | @uppy/xhr-upload | 3.2.0 | | @uppy/locales | 3.2.0 | @uppy/zoom | 2.1.1 | | @uppy/onedrive | 3.1.1 | uppy | 3.8.0 | - @uppy/companion: increase max limits for remote file list operations (Mikael Finstad / transloadit#4417) - @uppy/xhr-upload: fix type in README.md (Top Master / transloadit#4416) - @uppy/core: improve performance of validating & uploading files (Mikael Finstad / transloadit#4402) - @uppy/provider-views: Concurrent file listing (Mikael Finstad / transloadit#4401) - @uppy/core,@uppy/locales,@uppy/provider-views: User feedback adding recursive folders take 2 (Mikael Finstad / transloadit#4399) - @uppy/dashboard: Single File Mode: fix layout and make optional (Artur Paikin / transloadit#4374) - @uppy/informer: add a check in `TransitionGroup` when component is null (Juan Belej / transloadit#4410) - meta: Fix logos in all the readmes (Artur Paikin / transloadit#4407) - meta: fix logo in readme (Kid / transloadit#4403)

mifi added 6 commits April 4, 2023 23:09

show how many files are added when loading

eff6876

remake of #4388

add french (cherry pick)

d67a4a5

implement concurrent file listing

435c71b

refactor / fix lint

133600d

refactor/reduce duplication

47e2980

pull out totals validation

966d3e7

don't do it for every file added, as it's very slow instead do the check at the end when all files are added. this allows us to easily work with 10k+ files fixes #4389

aduh95 reviewed Apr 5, 2023

View reviewed changes

packages/@uppy/core/src/Uppy.js Outdated Show resolved Hide resolved

packages/@uppy/core/src/Uppy.js Outdated Show resolved Hide resolved

Murderlon reviewed Apr 5, 2023

View reviewed changes

packages/@uppy/core/src/Restricter.js Outdated Show resolved Hide resolved

packages/@uppy/core/src/Uppy.js Outdated Show resolved Hide resolved

packages/@uppy/dashboard/src/components/FileList.jsx Show resolved Hide resolved

Murderlon changed the title ~~Performance improvements~~ @uppy/core: improve performance of validating files Apr 5, 2023

mifi and others added 4 commits April 6, 2023 13:11

Update packages/@uppy/core/src/Uppy.js

94a3385

Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>

make restricter.validate validate everything

9f53afe

instead make more specific methods for sub-validation also rename validateTotals to validateAggregateRestrictions

fix performance issue reintroduced

4142923

Murderlon reviewed Apr 6, 2023

View reviewed changes

packages/@uppy/core/src/Uppy.js Outdated Show resolved Hide resolved

packages/@uppy/core/src/Uppy.js Outdated Show resolved Hide resolved

packages/@uppy/core/src/Uppy.js Outdated Show resolved Hide resolved

mifi added 5 commits April 7, 2023 00:13

improvements

a8fb99c

- show "%{count} additional restrictions were not fulfilled" for any restriction errors more than 4 - refactor/rename methods - improve ghost logic/comments

fix e2e build issue

ff3e580

try to upgrade cypress

b43864f

maybe it fixes the error

Revert "fix e2e build issue"

a84e3f6

This reverts commit ff3e580.

upgrade parcel

00bd146

mifi changed the title ~~@uppy/core: improve performance of validating files~~ @uppy/core: improve performance of validating & uploading files Apr 10, 2023

Murderlon reviewed Apr 11, 2023

View reviewed changes

packages/@uppy/provider-views/src/ProviderView/ProviderView.jsx Show resolved Hide resolved

packages/@uppy/utils/src/fileFilters.js Show resolved Hide resolved

Murderlon reviewed Apr 11, 2023

View reviewed changes

packages/@uppy/core/src/Uppy.js Show resolved Hide resolved

packages/@uppy/core/src/Uppy.js Outdated Show resolved Hide resolved

packages/@uppy/core/src/Uppy.js Outdated Show resolved Hide resolved

mifi added 3 commits April 12, 2023 14:27

move mutation logic to end

2474b16

remove FileRestrictionError

5ebda51

merge it with RestrictionError

fix silly bug

33311d8

looks like the e2e tests are doing its job 👏

mifi mentioned this pull request Apr 12, 2023

Fix logic with adding folders toast message #4414

Closed

2 tasks

Murderlon requested review from aduh95 and arturi April 12, 2023 16:32

Murderlon approved these changes Apr 13, 2023

View reviewed changes

arturi reviewed Apr 14, 2023

View reviewed changes

arturi approved these changes Apr 14, 2023

View reviewed changes

mifi added 2 commits April 15, 2023 23:23

Merge branch 'main' into concurrent-file-listing

bd6e0d6

Merge branch 'concurrent-file-listing' into performance-improvements

73c21ba

Base automatically changed from concurrent-file-listing to main April 15, 2023 15:42

mifi merged commit 90f7fb9 into main Apr 15, 2023

mifi deleted the performance-improvements branch April 15, 2023 15:50

github-actions bot mentioned this pull request Apr 18, 2023

Release: uppy@3.8.0 #4418

Merged

trabab mentioned this pull request May 18, 2023

[Snyk] Upgrade aws-sdk from 2.701.0 to 2.1366.0 trabab/uppy#674

Open

This was referenced May 20, 2023

[Snyk] Upgrade aws-sdk from 2.701.0 to 2.1368.0 trabab/uppy#675

Open

[Snyk] Upgrade aws-sdk from 2.701.0 to 2.1370.0 trabab/uppy#678

Open

trabab mentioned this pull request May 26, 2023

[Snyk] Upgrade aws-sdk from 2.701.0 to 2.1372.0 trabab/uppy#679

Open

This was referenced May 29, 2023

[Snyk] Upgrade aws-sdk from 2.701.0 to 2.1373.0 trabab/uppy#680

Open

[Snyk] Upgrade aws-sdk from 2.701.0 to 2.1374.0 trabab/uppy#681

Open

trabab mentioned this pull request Jun 1, 2023

[Snyk] Upgrade aws-sdk from 2.701.0 to 2.1376.0 trabab/uppy#682

Open

snyk-bot mentioned this pull request Jun 2, 2023

[Snyk] Upgrade aws-sdk from 2.701.0 to 2.1377.0 trabab/uppy#683

Open

trabab mentioned this pull request Jun 6, 2023

[Snyk] Upgrade aws-sdk from 2.701.0 to 2.1378.0 trabab/uppy#684

Open

snyk-bot mentioned this pull request Jun 7, 2023

[Snyk] Upgrade aws-sdk from 2.701.0 to 2.1379.0 trabab/uppy#685

Open

trabab mentioned this pull request Jun 9, 2023

[Snyk] Upgrade aws-sdk from 2.701.0 to 2.1380.0 trabab/uppy#686

Open

mifi mentioned this pull request Sep 13, 2023

UI slowness at many files (10k) #4679

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

@uppy/core: improve performance of validating & uploading files #4402

@uppy/core: improve performance of validating & uploading files #4402

mifi commented Apr 5, 2023 •

edited

Loading

Murderlon left a comment

mifi commented Apr 6, 2023

socket-security bot commented Apr 10, 2023 •

edited

Loading

mifi commented Apr 10, 2023

mifi commented Apr 10, 2023

Murderlon left a comment

mifi commented Apr 13, 2023

arturi commented Apr 13, 2023

arturi Apr 14, 2023

mifi Apr 15, 2023

arturi commented Apr 14, 2023

mifi commented Apr 15, 2023

@uppy/core: improve performance of validating & uploading files #4402

@uppy/core: improve performance of validating & uploading files #4402

Conversation

mifi commented Apr 5, 2023 • edited Loading

Murderlon left a comment

Choose a reason for hiding this comment

mifi commented Apr 6, 2023

socket-security bot commented Apr 10, 2023 • edited Loading

mifi commented Apr 10, 2023

mifi commented Apr 10, 2023

Murderlon left a comment

Choose a reason for hiding this comment

mifi commented Apr 13, 2023

arturi commented Apr 13, 2023

arturi Apr 14, 2023

Choose a reason for hiding this comment

mifi Apr 15, 2023

Choose a reason for hiding this comment

arturi commented Apr 14, 2023

mifi commented Apr 15, 2023

mifi commented Apr 5, 2023 •

edited

Loading

socket-security bot commented Apr 10, 2023 •

edited

Loading