-
Notifications
You must be signed in to change notification settings - Fork 2.2k
WIP AngularFireStorage #963
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
Conversation
@jamesdaniels, when it will be in beta? I would help with testing. |
I am looking forward to this release! |
This is what I did for getting something from storage. import { Pipe, PipeTransform, Inject } from '@angular/core';
import { FirebaseApp } from 'angularfire2';
import * as firebase from 'firebase';
@Pipe({
name: 'fromStorage'
})
export class FromStoragePipe implements PipeTransform {
private storageRef: any;
private bucket: string = 'gs://path-to-bucket';
constructor(
@Inject(FirebaseApp) firebaseApp: any
) {
this.storageRef = firebase.storage(firebaseApp).refFromURL(this.bucket);
}
transform(url: string): Promise<string> {
if (!url) {
return Promise.resolve('');
}
if (url && url.includes('http')) {
return Promise.resolve(url);
}
return this.storageRef.child(url).getDownloadURL();
}
} |
This looks good! When will it be merged? 👍 |
I'm new to helping on open source projects but would be glad to assist with tests. Setting up on this branch, I ran into the Typescript 2.4 error that I think you already fixed with #1076 . Unless I missed something in the commit history on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding comment pointing to a type error.
|
||
export function FirebaseUploadTaskFactory (uploadTask: UploadTask): FirebaseUploadTaskObservable<UploadTaskSnapshot> { | ||
|
||
const objectObservable = new FirebaseUploadTaskObservable((obs: Observer<UploadTaskSnapshot>) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm getting a type error here: Type 'R' is not assignable to type 'UploadTaskSnapshot'.
I'm still poking at it but maybe someone else has insight into RxJS observer patterns.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'll get this back up to speed with master to fix shortly. Was letting David get in his systemjs fixes before piling on.
@jamesdaniels whats the status on this? Do you need a hand? |
Want to see it soon. |
I would be keen to see this released, let me know if there's anything I can help with. |
Closing due to 5.0.0 refactor. We will have a better solution in the near future. |
Checklist
npm install
,npm run build
, andnpm test
run successfully? yesDescription
Adding
AngularFireStorage
as proposed in #940I have to extend test coverage over this, but wanted to open my WIP to critique.
Further I adding the start of
AngularFireMessaging
. It only has.messaging
which points towards the initializedfirebase.messaging.Messaging
object right now, for consistency. I will flush this out in another proposal/PR.Code sample