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

fix native js call to super #582

Merged
merged 3 commits into from Sep 14, 2019

Conversation

@formigarafa
Copy link
Contributor

commented Jun 19, 2019

Description

Fixing an error on firebaseApp service loading.

Assertion Failed: You must call `this._super(...arguments);` when overriding `init` on a framework object. 
Please update <app@service:firebase-app::ember180> to call `this._super(...arguments);` from `init`.

Note that I had an issue with typescript compilation. Initially I followed Ember conventions by using super.init(...arguments); like they do here: https://github.com/emberjs/ember.js/blob/master/packages/@ember/-internals/routing/lib/services/router.ts#L64-L65

Then I have tried to workaround the typescript error but IMHO I think the way Ember did is the correct.

@googlebot

This comment has been minimized.

Copy link

commented Jun 19, 2019

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

1 similar comment
@googlebot

This comment has been minimized.

Copy link

commented Jun 19, 2019

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@formigarafa

This comment has been minimized.

Copy link
Contributor Author

commented Jun 19, 2019

@googlebot

This comment has been minimized.

Copy link

commented Jun 19, 2019

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

1 similar comment
@googlebot

This comment has been minimized.

Copy link

commented Jun 19, 2019

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

formigarafa added 2 commits Jun 19, 2019
@patilharshal16
Copy link

left a comment

Is it necessary to pass arguments in init?

@formigarafa

This comment has been minimized.

Copy link
Contributor Author

commented Jul 14, 2019

I don't know. but It seems Ember's base classes pass or used to pass something there.
If we don't need to pass any argument here, then Ember base classes are wrong.
We can see the type mismatch by the error I reported above.

@oskarrough

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2019

Also having this error. I tested and changing the line to super.init(...arguments) works here.

Can we merge? ⚡️

@oskarrough oskarrough referenced this pull request Jul 19, 2019
6 of 17 tasks complete
@flecno

This comment has been minimized.

Copy link

commented Sep 12, 2019

I'd like to upgrade too. I would also be happy about a merge.

@jamesdaniels jamesdaniels merged commit 3d72487 into firebase:master Sep 14, 2019

2 checks passed

cla/google All necessary CLAs are signed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.