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

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

Open
wants to merge 2 commits into
base: 3.1.x
Choose a base branch
from

Conversation

fabiofabbri84
Copy link

@fabiofabbri84 fabiofabbri84 commented Jul 11, 2022

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 . 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:/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:/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:/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:/cimatti/cordova-electron-v32proposal-demo-plugin

cordova plugin remove cordova-electron-v3-demo-plugin
cordova plugin add https:/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

  • 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 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 incapsulated in an Error object. See electron/electron#24427

This patch adds a success and error callback parameter for the plugin action, that can be used to implement the action function in a way more coherent with the Cordova architecture, and allowing to bypass the error callback parameter issue.

However, for backward compatibility, the action must call the callback parameters before returning (or before fulfilling the returned promise), otherwise it will work as in previous 3.x cordova-electron implementations, calling the success callback with the returned value or calling the error callback with an Error object.

Finally, _cdvElectronIPC.exec checks if success and error parameters are actually functions before calling them, in order to avoid annoying error log message 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