baumblatt / capacitor-firebase-auth

Capacitor Firebase Authentication Plugin
MIT License
261 stars 130 forks source link

Cannot convert value of type 'NSError' to expected argument type 'String?' #66

Open vichanse opened 4 years ago

vichanse commented 4 years ago

I'm having this error when i try to build the project with the capacitor-firebase-atuh plugin installed. I get this error in the signOut function in Plugin.swift file at this line call.reject("Error signing out: %@", signOutError)

The second argument of the reject function is a String not an NSError

func reject( message: String, code: String? = nil, error: Error? = nil, data: PluginCallErrorData = [:]) { errorHandler(CAPPluginCallError(message: message, code: code, error: error, data: data)) }

FYI: When i remove that second argument, the build succeed...

mckelveygreg commented 4 years ago

It looks like they changed the signature of that call in capacitor 2.0 🤷‍♂ https://github.com/ionic-team/capacitor/commit/f93c35481afeade8fcd5689a5aa605650455dfcd#diff-6b691083c4b578ecb735584c4dede9d2L84-L87

In the Plugin.swift file, I moved the error call to the third argument, and put nil as the second to follow the new api.

} catch let signOutError as NSError {
            print ("Error signing out: %@", signOutError)
            call.reject("Error signing out: %@", nil, signOutError)
        }
mckelveygreg commented 4 years ago

On second look, i think it was a problem with the template string. I made a PR with a change that works for me and that still gets the error through: #67

vichanse commented 4 years ago

I would go for your first solution. The second parameter expects an error code so putting nil seems to be the right solution IMO. I tested it and it works. So 👍 for your first solution

mckelveygreg commented 4 years ago

However, both of those options are optional, and the %@ is expecting an argument that it won't be getting anyway I think. However, this is the first swift I've written 🙃

Regardless, I don't think git tracks pods? So would we need something to be merged to not break new builds?

gazaay commented 4 years ago

It looks like they changed the signature of that call in capacitor 2.0 🤷‍♂ ionic-team/capacitor@f93c354#diff-6b691083c4b578ecb735584c4dede9d2L84-L87

In the Plugin.swift file, I moved the error call to the third argument, and put nil as the second to follow the new api.

} catch let signOutError as NSError {
            print ("Error signing out: %@", signOutError)
            call.reject("Error signing out: %@", nil, signOutError)
        }

Thanks I just use patch-package with your suggestion and fix my breaking build in AppFlow.

baumblatt commented 4 years ago

Hello,

The fix is available on release v0.3.1.

Please let me know about your tests and if we can close this issue.

If you like this plugin, please don't forget to star the project to help others to find it.

Best regards, Bernardo Baumblatt