cordova-electron icon indicating copy to clipboard operation
cordova-electron copied to clipboard

feat: improve support for plugins code running in main, keeping backward compatibility

Open fabiofabbri84 opened this issue 2 years ago • 0 comments

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 https://github.com/electron/electron/issues/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 https://github.com/apache/cordova-electron/issues/214 . To maintain backward compatibility, I don't address this in this merge request, but I have a proposal for cordova-electron 4.x

Description

To fix this, on the main side:

  • A promise is returned
  • two functions (success and error) are passed as additional parameters to the plugin function.
  • If one of these two functions is called before the plugin returns a value (or before fulfilling a returned promise), the promise will be resolved returning an object, with boolean property "success" (true if success was called, false if error was called) and property "result" with the value that was passed to the callback function.
  • If the function returns a value (not an unfulfilled promise), the promise will be resolved returning an object as above, with property "result" set to this value, and "success" true.
  • If a rejected promise is returned, or an error is thrown, the promise will be rejected, for backward compatibility

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 as before, to maintain backward compatibility.
  • Before calling "success" or "error" callback, it should be checked if they are functions, to avoid annoying error messages if they were not defined.

Testing

I made demo plugin with cordova-electron 3.x support, based on plugman example. You can find it here: https://github.com/cimatti/cordova-electron-v3-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-v3-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 3.2 proposal:

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

Calling again the JavaScript console functions above, you can see that backward compatibility is maintained, but calling DemoPlugin.coolMethod omitting callback parameters, you don't get errors.

Now you can test my cordova-electron 3.2 proposal with a plugin supporting it, available at ttps://github.com/cimatti/cordova-electron-v32proposal-demo-plugin

cordova plugin remove cordova-electron-v3-demo-plugin
cordova plugin add https://github.com/cimatti/cordova-electron-v32proposal-demo-plugin
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 plain cordova-electron 3.1 to check how is possible to make a plugin compatible with both cordova-electon 3.1 and my proposal for cordova-electron 3.2

cordova platform remove electron
cordova platform add [email protected]
cordova run electron --nobuild

Checklist

  • [x] 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

fabiofabbri84 avatar Jul 11 '22 14:07 fabiofabbri84