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

breaking: improve support for plugins code running in main, fixing quirks of 3.x, and making it more coherent with Cordova APIs #239

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fabiofabbri84
Copy link

Motivation and Context

  • In cordova-electron 3.x, the interface between code running in the main process and in the renderer, is based purely on ipcRenderer, and the "success" callback is called when a value is returned, and "error" callback is called if an exception is thrown or a rejected promise is returned.
  • However, using this method, the object thrown or the rejected promise parameter is converted to String and encapsulated in an error, see ipcRenderer.invoke can't handle custom errors electron/electron#24427
  • usually, Cordova APIs relies on "success" and "error" callbacks, to which you can pass an argument, so I think it's better to keep this standard
  • A quirk I noticed investigating this, is that the plugin version receives the parameters encapsulated twice in an Array, see also Framework functions receive arguments boxed twice in an Array #214 .
  • As the standard used in cordova-electron 3.x is not documented and has some improvable quirks, this might be a good moment to make it more coherent with other Cordova APIs, at the same time allowing plugin developers to support both 3.x and 4.x standards (and even 3.2 if feat: improve support for plugins code running in main, keeping backward compatibility #238 is accepted)

Description

On the main side:

  • "...args" is replaced with "args" to address Framework functions receive arguments boxed twice in an Array #214
  • A promise is returned
  • the plugin action is called with arguments (success, error, args), to make it more coherent with other Cordova APIs and to make it easy to check if we are using 4.x standard (typeof first_argument == 'function') or 3.x standard (otherwise first argument is an array containing an array of arguments)
  • the first time success or error functions are called, the promise is resolved, passing an object with property "success" true or false depending on the function called, and property "result" containing the parameter passed to the function.

On the renderer side:

  • An object is expected to be returned from the main. If it's property "success" is true, the success callback is called passing property result as parameter, otherwise error is called the same way.
  • In case of error, the error value is passed to the error callback, but this should not happen.
  • Before calling "success" or "error" callback, we check if they actually are functions, to avoid annoying error messages if they were not defined.

Testing

I made demo plugin, based on plugman example, that supports this proposal, cordova-electron 3.1, and my proposal for cordova-electron 3.2. You can find it here: https://github.com/cimatti/cordova-electron-v4proposal-demo-plugin

You can create a demo project using this plugin with these commands:

cordova create mytest
cd mytest
cordova platform add [email protected]
cordova plugin add https://github.com/cimatti/cordova-electron-v4proposal-demo-plugin
cordova run electron --nobuild

On the electron app console, you can call the following JavaScript commands to test the problems with the current implementation:

// Defining the callbacks
var successCB = function(result){console.log("success");console.log(result)}
var errorCB = function(e){console.log("error");console.log(e)}

DemoPlugin.coolMethod('',successCB) // should do nothing
// With cordova-electron 3.x you get an uncaught exception in the log if success or error callback functions are invoked but are omitted from the arguments

DemoPlugin.coolMethod('',successCB, errorCB) // should log 'error' and the error message string
// With cordova-electron 3.x the error function argument is always converted to string and encapsulated in an Error object

DemoPlugin.coolMethod('hi', successCB, errorCB)
// should log 'success' and 'hi'

Now you can use these commands to replace cordova-electron 3.1 with my 4.0 proposal:

cordova platform remove electron
cordova platform add https://github.com/cimatti/cordova-electron.git#4.0.x-proposal
cordova run electron --nobuild

Calling again the JavaScript console functions above, you can see that everything is running as expected

Now you can also test this demo plugin with my cordova-electron 3.2 proposal (see #238 ) to check how is possible to make a plugin compatible with cordova-electon 3.1 and my proposals for cordova-electron 3.2 and 4.0

cordova platform remove electron
cordova platform add https://github.com/cimatti/cordova-electron.git#3.2.x-proposal
cordova run electron --nobuild

Checklist

  • I've run the tests to see all new and existing tests pass
  • I added automated test coverage as appropriate for this change
  • Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
  • If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
  • I've updated the documentation if necessary

…cdv-plugin-exec

The main goal was to allow passing to the error callback an unaltered copy of the argument, as with the previous implementation you have to throw an exception or return a rejected promise, and this is forwarded to the ipcRenderer.invoke as a string encapsulated in an Error object. See electron/electron#24427

Moreover, with the previous implementation, args for the action were encapsulated twice in an Array ( see apache#214 ). To make it more coherent with Cordova standards, and to allow backward support easily with a simple check, the first and the second arguments are success and error functions, and the third argument is the args array. So if the first argument is a function, we are using this cordova-electron~4 proposal, otherwise we are using cordova-electron~3, and it is the "arguments" array.

Finally, _cdvElectronIPC.exec checks if success and error parameters are actually functions before calling them, in order to avoid annoying error log messages if they were not defined.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant