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

getBytes #4973

Closed
wants to merge 9 commits into from
Closed

getBytes #4973

wants to merge 9 commits into from

Conversation

schmidt-sebastian
Copy link
Contributor

@schmidt-sebastian schmidt-sebastian commented Jun 3, 2021

Adds getBytes, getBlob and getStream APIs which allows file downloads without a public download URL. These bytes can then be used directly as an image source.

This PR contains a bunch of refactors to make this possible:

  • There are now three types of Connections. Connection<string>, Connection<ArrayBuffer> and Connection<ReadableStream>.
  • Each connection type has a distinct implementation for Node and Browsers.
  • There is no more ConnectionPool. ConnectionPool mostly seemed to be used to facilitate dependency injection for unit tests and never re-used connections. Instead, I added manual global overrides that provide the same functionality. This is not as elegant as before, but it allows the unused connections to be tree-shaken.

Fixes #76

@changeset-bot
Copy link

changeset-bot bot commented Jun 3, 2021

⚠️ No Changeset found

Latest commit: 1099618

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jun 3, 2021

Binary Size Report

Affected SDKs

  • @firebase/app

    Type Base (0a91a90) Head (2892d3b) Diff
    browser 11.0 kB 11.0 kB +1 B (+0.0%)
    esm2017 9.80 kB 9.80 kB +1 B (+0.0%)
    lite 8.93 kB 8.93 kB +1 B (+0.0%)
    lite-esm2017 7.93 kB 7.93 kB +1 B (+0.0%)
    main 10.1 kB 10.1 kB +1 B (+0.0%)
    module 11.0 kB 11.0 kB +1 B (+0.0%)
    react-native 9.86 kB 9.86 kB +1 B (+0.0%)
  • @firebase/firestore

    Type Base (0a91a90) Head (2892d3b) Diff
    browser 285 kB 285 kB +168 B (+0.1%)
    esm2017 227 kB 227 kB +4 B (+0.0%)
    module 285 kB 285 kB +168 B (+0.1%)
    react-native 227 kB 227 kB +4 B (+0.0%)
  • @firebase/firestore-compat

    Type Base (0a91a90) Head (2892d3b) Diff
    browser 29.0 kB 29.0 kB +1 B (+0.0%)
    main 38.2 kB 38.2 kB +1 B (+0.0%)
    module 29.0 kB 29.0 kB +1 B (+0.0%)
    react-native 28.7 kB 28.7 kB +1 B (+0.0%)
  • @firebase/firestore/bundle

    Type Base (0a91a90) Head (2892d3b) Diff
    browser 291 kB 291 kB +5 B (+0.0%)
    esm2017 177 kB 177 kB +5 B (+0.0%)
    main 528 kB 528 kB +1 B (+0.0%)
    module 291 kB 291 kB +5 B (+0.0%)
    react-native 177 kB 177 kB +5 B (+0.0%)
  • @firebase/firestore/memory

    Type Base (0a91a90) Head (2892d3b) Diff
    browser 217 kB 217 kB +4 B (+0.0%)
    esm2017 173 kB 173 kB +4 B (+0.0%)
    module 217 kB 217 kB +4 B (+0.0%)
    react-native 173 kB 173 kB +4 B (+0.0%)
  • @firebase/firestore/memory-bundle

    Type Base (0a91a90) Head (2892d3b) Diff
    browser 225 kB 225 kB +5 B (+0.0%)
    esm2017 177 kB 177 kB +5 B (+0.0%)
    main 322 kB 322 kB +1 B (+0.0%)
    module 225 kB 225 kB +5 B (+0.0%)
    react-native 177 kB 177 kB +5 B (+0.0%)
  • @firebase/storage

    Type Base (0a91a90) Head (2892d3b) Diff
    browser 64.1 kB 66.4 kB +2.31 kB (+3.6%)
    esm2017 55.2 kB 55.6 kB +391 B (+0.7%)
    main 55.8 kB 56.1 kB +300 B (+0.5%)
    module 64.1 kB 66.4 kB +2.31 kB (+3.6%)
  • @firebase/storage-compat

    Type Base (0a91a90) Head (2892d3b) Diff
    main 29.3 kB 29.4 kB +118 B (+0.4%)
  • @firebase/storage-exp

    Type Base (0a91a90) Head (2892d3b) Diff
    browser 51.9 kB 54.9 kB +2.97 kB (+5.7%)
    main 53.3 kB 54.6 kB +1.31 kB (+2.5%)
    module 51.9 kB 54.9 kB +2.97 kB (+5.7%)
  • firebase

    Type Base (0a91a90) Head (2892d3b) Diff
    firebase-app.js 22.1 kB 22.1 kB +3 B (+0.0%)
    firebase-firestore.js 337 kB 337 kB +4 B (+0.0%)
    firebase-firestore.memory.js 271 kB 271 kB +4 B (+0.0%)
    firebase-performance-standalone.es2017.js 73.5 kB 73.5 kB +2 B (+0.0%)
    firebase-performance-standalone.js 49.9 kB 49.9 kB +2 B (+0.0%)
    firebase-storage.js 45.0 kB 46.4 kB +1.41 kB (+3.1%)
    firebase.js 896 kB 897 kB +1.42 kB (+0.2%)

Test Logs

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jun 3, 2021

Size Analysis Report

Affected Products

No changes between base commit (0a91a90) and head commit (2892d3b).

@github-actions
Copy link
Contributor

github-actions bot commented Aug 13, 2021

Changeset File Check ⚠️

  • Changeset formatting error in following file:
    Some packages have been changed but no changesets were found. Run `changeset add` to resolve this error.
    If this change doesn't need a release, run `changeset add --empty`.
    

This adds a Node and a Browser specific build so we can have getStream() in Node and getBlob() in browsers
return super
.send(url, method, body, headers)
.then(() => (this.xhr_.response as Blob).arrayBuffer())
.then(data => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need to ensure this never throws

@firebase firebase locked and limited conversation to collaborators Nov 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
doc-changes PRs that affect docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FR: Alternative to getDownloadURL that doesn't expose a public download URL
4 participants