-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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: need to convert instance type from the JS class to the wrapper class #1956
Conversation
@@ -687,21 +663,53 @@ export class BaseClass { | |||
* | |||
* @return {Observable<any>} | |||
*/ | |||
@CordovaInstance({ |
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 see that you've removed some decorators. I suggest you replace them with CordovaCheck
and InstanceCheck
decorators. These two decorators check if the plugin/objectInstance exists before executing the functionality. This helps eliminate runtime errors when we're developing outside of a Cordova platform.
To use the decorators, you have to give them a configuration that specifies what to return in case Cordova is not available. You can return an empty promise, empty observable, or null
.
@CordovaCheck() // returns promise
@CordovaCheck({ observable: true }) // returns observable
@CordovaCheck({ sync: true }) // returns `null`
Thank you for your suggestion. I have been kind of busy in couple of days. I will update this later. |
add: BaseClass.destroy() add: getId() method for all instance classes
Sorry, after the discussion with ionic team, I don't support the |
Since the problem has solved at once, I reopen this PR. |
@ihadeed ping |
Please merge this pull request, and release as
@ionic-native/google-maps
4.2.2.And please specify the dependency module
@ionic-native/core
4.2.0