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.

Sign up for GitHub

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

Jump to bottom

examples/aws-nodejs: showcase an example without preflight requests #4516

Merged
merged 2 commits into from
Jun 27, 2023
Merged

examples/aws-nodejs: showcase an example without preflight requests #4516

merged 2 commits into from
Jun 27, 2023

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Jun 21, 2023

Using url-encoded body instead of JSON allows the browser to skip the CORS preflight request, speeding up the upload.

@aduh95 aduh95 requested a review from Murderlon June 21, 2023 19:17
Murderlon
Murderlon requested changes Jun 22, 2023
View reviewed changes
Copy link
Member

@Murderlon Murderlon 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 an optimisation I'd rather not have as an example, but something that is included in Uppy if it makes sense. Any reason why we wouldn't want this?

@aduh95
Copy link
Contributor Author

aduh95 commented Jun 22, 2023

See #4462 (comment)

@Murderlon
Copy link
Member

I'm not sure I follow. We avoid preflight requests when companion is used, but this is still needed if you use your own backend? Can't we integrate that as well? And it seems this aws-nodejs is now an example which is actually multiple examples. regular, multipart, and this sneaky change which isn't obvious to people?

@aduh95
Copy link
Contributor Author

aduh95 commented Jun 22, 2023 •
edited
Loading

We avoid preflight requests when companion is used, but this is still needed if you use your own backend?

No we don't, and we can't currently because Transloadit.com expects a header to be present (or it will assume the request comes from a very old version of Uppy), and that forces a CORS preflight for all requests to Companion.

And it seems this aws-nodejs is now an example which is actually multiple examples. regular, multipart, and this sneaky change which isn't obvious to people?

Combining both examples is a goal, but not in this PR. Using URL-encoded instead of JSON seems like the right thing to do for anyone implementing a Companion-less Uppy setup, so I think our example should showcase it, but no strong feelings.

@Murderlon
Copy link
Member

Using URL-encoded instead of JSON seems like the right thing to do for anyone implementing a Companion-less Uppy setup, so I think our example should showcase it, but no strong feelings.

It's just odd that this is never mentioned in the docs and only if you happen to stumble on this example. And if it's always "the right thing to do", why can't we check the response type and do this conditionally internally?

@aduh95
Copy link
Contributor Author

aduh95 commented Jun 26, 2023

Using URL-encoded instead of JSON seems like the right thing to do for anyone implementing a Companion-less Uppy setup, so I think our example should showcase it, but no strong feelings.

It's just odd that this is never mentioned in the docs and only if you happen to stumble on this example. And if it's always "the right thing to do", why can't we check the response type and do this conditionally internally?

Do you mean in Companion? Sure but that won’t simplify this example (the server side changes are quite minimal tbh).

@aduh95 aduh95 merged commit 435cb8b into transloadit:main Jun 27, 2023
Murderlon added a commit that referenced this pull request Jul 5, 2023
@Murderlon
Merge branch 'main' into fix-throttle-global-state
34ef94a 
* main:
  @uppy/aws-s3-multipart: fix Golden Retriever integration (#4526)
  examples/aws-nodejs: merge multipart and non-multipart examples (#4521)
  @uppy/companion: bump semver from 7.3.7 to 7.5.3 (#4529)
  @uppy/aws-s3-multipart: add types to internal fields (#4535)
  examples/aws-nodejs: update README (#4534)
  examples/aws-nodejs: showcase an example without preflight requests (#4516)
  @uppy/aws-s3-multipart: fix pause/resume (#4523)
  @uppy/status-bar: fix ETA when Uppy recovers its state (#4525)
  @uppy/aws-s3-multipart: fix resume single-chunk multipart uploads (#4528)
  @uppy/companion: fix part listing in s3 (#4524)
  example/aws-php: make it forward-compatible with the next Uppy major (#4522)
  @uppy/golden-retriever: refactor to modernize the codebase (#4520)
  examples/aws-nodejs: upgrade to AWS-SDK v3 (#4515)
  @uppy/companion: implement refresh for authentication tokens (#4448)
  @uppy/aws-s3-multipart: disable pause/resume for remote uploads in the UI (#4500)
  @uppy/tus: retry on 423 HTTP error code (#4512)
@github-actions github-actions bot mentioned this pull request Jul 6, 2023
Merged
github-actions bot added a commit that referenced this pull request Jul 6, 2023
@github-actions
Release: uppy@3.11.0 (#4550)
ded685e 
| Package                | Version | Package                | Version |
| ---------------------- | ------- | ---------------------- | ------- |
| @uppy/aws-s3           |   3.2.1 | @uppy/golden-retriever |   3.1.0 |
| @uppy/aws-s3-multipart |   3.4.1 | @uppy/status-bar       |   3.2.1 |
| @uppy/companion        |   4.6.0 | @uppy/tus              |   3.1.2 |
| @uppy/companion-client |   3.2.0 | @uppy/xhr-upload       |   3.3.1 |
| @uppy/core             |   3.3.0 | uppy                   |  3.11.0 |

- @uppy/companion: fix infinite recursion in uploader test (Mikael Finstad / #4536)
- @uppy/xhr-upload: export `Headers` type (Masum ULU / #4549)
- @uppy/aws-s3-multipart: increase priority of abort and complete (Stefan Schonert / #4542)
- @uppy/aws-s3: fix remote uploads (Antoine du Hamel / #4546)
- meta: use `corepack yarn` instead of `npm` to launch E2E (Antoine du Hamel / #4545)
- @uppy/aws-s3-multipart: fix upload retry using an outdated ID (Antoine du Hamel / #4544)
- @uppy/status-bar: remove throttled component (Artur Paikin / #4396)
- @uppy/aws-s3-multipart: fix Golden Retriever integration (Antoine du Hamel / #4526)
- examples/aws-nodejs: merge multipart and non-multipart examples (Antoine du Hamel / #4521)
- @uppy/companion: bump semver from 7.3.7 to 7.5.3 (dependabot[bot] / #4529)
- @uppy/aws-s3-multipart: add types to internal fields (Antoine du Hamel / #4535)
- examples/aws-nodejs: update README (Antoine du Hamel / #4534)
- examples/aws-nodejs: showcase an example without preflight requests (Antoine du Hamel / #4516)
- @uppy/aws-s3-multipart: fix pause/resume (Antoine du Hamel / #4523)
- @uppy/status-bar: fix ETA when Uppy recovers its state (Antoine du Hamel / #4525)
- @uppy/aws-s3-multipart: fix resume single-chunk multipart uploads (Antoine du Hamel / #4528)
- @uppy/companion: fix part listing in s3 (Antoine du Hamel / #4524)
- example/aws-php: make it forward-compatible with the next Uppy major (Antoine du Hamel / #4522)
- @uppy/golden-retriever: refactor to modernize the codebase (Antoine du Hamel / #4520)
- examples/aws-nodejs: upgrade to AWS-SDK v3 (Antoine du Hamel / #4515)
- @uppy/companion: implement refresh for authentication tokens (Mikael Finstad / #4448)
- @uppy/aws-s3-multipart: disable pause/resume for remote uploads in the UI (Artur Paikin / #4500)
- @uppy/tus: retry on 423 HTTP error code (Antoine du Hamel / #4512)
Murderlon added a commit that referenced this pull request Jul 10, 2023
@Murderlon
Merge branch 'main' into provider-view-load-all-files
4ae3544 
* main: (24 commits)
  Add missing pt-BR locales for ImageEditor plugin (#4558)
  Release: uppy@3.11.0 (#4550)
  @uppy/companion: fix infinite recursion in uploader test (#4536)
  @uppy/xhr-upload: export `Headers` type (#4549)
  @uppy/aws-s3-multipart: increase priority of abort and complete (#4542)
  @uppy/aws-s3: fix remote uploads (#4546)
  meta: use `corepack yarn` instead of `npm` to launch E2E (#4545)
  @uppy/aws-s3-multipart: fix upload retry using an outdated ID (#4544)
  @uppy/status-bar: remove throttled component (#4396)
  @uppy/aws-s3-multipart: fix Golden Retriever integration (#4526)
  examples/aws-nodejs: merge multipart and non-multipart examples (#4521)
  @uppy/companion: bump semver from 7.3.7 to 7.5.3 (#4529)
  @uppy/aws-s3-multipart: add types to internal fields (#4535)
  examples/aws-nodejs: update README (#4534)
  examples/aws-nodejs: showcase an example without preflight requests (#4516)
  @uppy/aws-s3-multipart: fix pause/resume (#4523)
  @uppy/status-bar: fix ETA when Uppy recovers its state (#4525)
  @uppy/aws-s3-multipart: fix resume single-chunk multipart uploads (#4528)
  @uppy/companion: fix part listing in s3 (#4524)
  example/aws-php: make it forward-compatible with the next Uppy major (#4522)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Murderlon Murderlon Murderlon approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@aduh95 @Murderlon