cordova-plugin-file icon indicating copy to clipboard operation
cordova-plugin-file copied to clipboard

Published 20 hours ago • verified publisher icon apache

feat(electron): add platform support

Open pjoriginal opened this issue 2 years ago • 5 comments

Platforms affected

Electron

Motivation and Context

This is required so that cordova electron can have a working file plugin

fixes #334 fixes #312 fixes #477 closes #371 closes #387

Description

~~The base implementation has two things.~~ ~~1. Scripts to add and remove node implementations done in files cdv-electron-main.js and cdv-electron-preload.js. Since the scripts look for comments, and remove code between the comments on plugin remove, we should be good as long as the dev doesn't mess those up. These scripts are added to support Context Isloation that has been encouraged in electron@^3.0. The fs required can be called from window.cordovaFilePlugin.function. I had to tweak some changes because the return value objects have their methods removed.~~

With the help of Erisu, I was able to update the code so that it doesn't require extra scripts. The electron implementation uses the framework keyword and uses backend node-js functions to implement stuff.

Testing

I've tested them on my personal projects. Will write an exclusive test project if necessary, Also, https://github.com/pjoriginal/cordova-file-test

Checklist

  • [X] I've run the tests to see all new and existing tests pass
  • [X] Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
  • [X] 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)
  • [x] I've updated the documentation if necessary

pjoriginal avatar Dec 13 '22 09:12 pjoriginal

The Android & iOS tests can be ignored because there are existing issues present that is causing them to fail, and that's been happening to some time. We know it's not caused by this PR.

Lint test should pass, which it currently does.

Will write an exclusive test project if necessary

Obviously it will be nice to have actual unit tests, but I won't personally reject a PR without unit tests. It can always be added later.

breautek avatar Jan 05 '23 00:01 breautek

I'm sorry. I accidentally clicked on "close this issue" while trying to comment on the issue.

The test project has been created. Will add unit tests soon

pjoriginal avatar Jan 10 '23 15:01 pjoriginal

@breautek added functionality and added electron stuff to tests where missing. Also fixed eslint.

pjoriginal avatar Apr 25 '23 07:04 pjoriginal

When will this be committed?

regnete avatar Aug 23 '23 11:08 regnete

Making chnages to make this compatible to electron@nightly

pjoriginal avatar Feb 21 '24 07:02 pjoriginal