feat(storage): getDownloadURL pipe

[jamesdaniels]

Checklist

• Issue number for this PR: #nnn (required)
• Docs included?: (yes/no; required for all API/functional changes)
• Test units included?: (yes/no; required)
• In a clean directory, yarn install, yarn test run successfully? (yes/no; required)

Description

Code sample

[googlebot]

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

[google-cla]

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

[googlebot]

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

[google-cla]

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

[AckerApple]

When looking over the code and files as a whole, sometimes it’s “Url” and sometimes it’s “URL”. Also, using a common name for a global item (pipes) such as “getDownloadURL” is asking for name collisions.

#2089 has been in production use several times over in company projects I’ve worked on.

Fun follow up question since you used all caps “URL”, I’d like to know what your variable would look like if you had to get a “url uuid”. Laughing thinking you would “getURLUUID” instead of “getUrlUuid”.

Sorry, it’s taken years since my PR for this feature and seems odd to see it turned into a perceived lesser flawed version.

[jamesdaniels]

@AckerApple Thanks for the candid feedback, ultimately I decided to go with what the API committee on storage chose for the method name on the storage reference getDownloadURL(), all caps since technically URL is a TLA. We also have pure-functions inbound which will also be import { getDownloadURL } from '@firebase/storage' and will probably mirror that API in AngularFire and RxFire, so best not to mix the cases IMO.

I could be convinced to prefix the pipe, perhaps getStorageDownloadURL or getCloudStorageDownloadURL before 6.1 final drops. What happens in Angular if there's a conflict? I haven't encountered this before. We decided not to prefix our new modular APIs, since you can easily rename in the import, e.g import { startAt as fsStartAt } from '@firebase/firestore'; import { startAt as dbStartAt } from '@firebase/database'; is there something similar or does it all explode? Seems like a common occurrence, especially if you pull in one of the popular pipe utility libraries.

I'm largely aiming for simplicity with the API here, hope you can understand.

[jamesdaniels]

It was a bit of a pain to get it to work in Ivy :( that could've been documented better on angular.io. Also what do you think of it building on top of the async pipe?

[AckerApple]

Thank you for explaining and giving care to my concerns.

I’ve come across two named pipes before and my memory strongly remembers both pipes are called BUT I can’t remember in what order and if one result feeds into the other. If I recall, you get no crash warning even, it just “happens”. Tried to briefly lookup but no time.

In my production version with my pipe, the async pipe has to be added to end of pipes like I see you have here. I don’t see any other way of doing it