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

Native electron support #371

Open
wants to merge 47 commits into
base: master
Choose a base branch
from
Open

Native electron support #371

wants to merge 47 commits into from

Conversation

zorn-v
Copy link

@zorn-v zorn-v commented Jan 19, 2020

Platforms affected

Electron

Motivation and Context

Use native nodejs functions for filesystem instead of writing files to chrome IndexedDB of electron app.
Closes #334 #312

Testing

https://github.com/zorn-v/cordova-electron-file-test

Checklist

  • nodeIntegration set to true after plugin install
  • resolveLocalFileSystemURL should resolve correct paths
  • readEntries
  • getFile
  • getFileMetadata
  • getMetadata
  • setMetadata
  • write
  • readAsText
  • readAsDataURL
  • readAsBinaryString
  • readAsArrayBuffer
  • removeRecursively
  • getDirectory
  • getParent
  • copyTo
  • moveTo
  • resolveLocalFileSystemURI
  • truncate

@zorn-v
Copy link
Author

zorn-v commented Jan 19, 2020

Currently I don't know how to execute native nodejs require for require('electron') because cordova override it

@zorn-v zorn-v requested review from stevengill and surajpindoria and removed request for stevengill January 22, 2020 11:20
@cyberplant
Copy link

Thanks @zorn-v , I'll check that, as I'm in the same situation as @miroslavvojtus and probably others..

I was not able to make it work, so I decided to go back to just electron and forget about cordova (and mobile devices...).

@pjoriginal
Copy link

@miroslavvojtus

You can use cordova plugin add https://github.com/zorn-v/cordova-plugin-file.git#electron
Or change it in package.json etc. to git+https://github.com/zorn-v/cordova-plugin-file.git#electron and run something like rm -rf plugins/cordova-plugin-file && npx cordova prepare

I tested this and it works like a charm.
The only error I get is when I get the filewriter and use the truncate function.
It throws a "missing command error". If it isn't used, everything works as expected

@zorn-v
Copy link
Author

zorn-v commented May 21, 2020

The only error I get is when I get the filewriter and use the truncate function.
It throws a "missing command error"

I'm just copy FileProxy.js from browser platform and implement all functions via native node. There was not truncate, but it is not problem to add it.

@antwal
Copy link

antwal commented Jun 19, 2020

Tested but not working; i have removed last cordova-plugin-file and added https://github.com/zorn-v/cordova-plugin-file.git#electron

plugin-file

@zorn-v
Copy link
Author

zorn-v commented Jun 20, 2020

You doing something wrong.
Try to remove node_modules, platforms, plugins and run npm i && npx cordova prepare

@Qualphey
Copy link

Try to remove node_modules, platforms, plugins and run npm i && npx cordova prepare

I'm using cordova 10.0.0 with cordova-electron 2.0.0 on Linux. When the plugin gets loaded it throws an error:

 Uncaught TypeError: Cannot read property 'app' of undefined

Source: src/electron/FileProxy.js:35

so it's this line:

const app = nodeRequire('electron').remote.app;

I've checked the result of console.log(nodeRequire('electron')) and it does not contain no properties like remote, app or anything similar. There is a webFrame property but it does not contain any functions that are used by the plugin.

Is this happening because I'm using a wrong cordova or cordova-electron version?

@zorn-v
Copy link
Author

zorn-v commented Dec 21, 2020

Seems in electron 10 remote property is disabled by default
https://github.com/electron/electron/blob/master/docs/breaking-changes.md#default-changed-enableremotemodule-defaults-to-false
I will add fix for that

@zorn-v
Copy link
Author

zorn-v commented Dec 21, 2020

Try to reinstall plugin and check again

@Qualphey
Copy link

Now I can't fetch the plugin anymore:

Discovered plugin "cordova-plugin-file". Adding it to the project
Failed to fetch plugin git+https://github.com/zorn-v/cordova-plugin-file.git#electron via registry.
Probably this is either a connection problem, or plugin spec is incorrect.
Check your connection and plugin name/version/URL.
CordovaError: CordovaError: Could not determine package name from output:
up to date, audited 421 packages in 6s

Same Check your connection and plugin name/version/URL error message when trying to add the plugin from file system. Is there something wrong with my configuration?

@Qualphey
Copy link

I've figured out that fetch was failing because I've upgraded nodejs to v15.5.0. After downgrading it back to v14.12.0 it started working again.
So I've managed to add the new version of your plugin, but unfortunately it is still dumping Error: exec proxy not found for :: File :: requestFileSystem.

@zorn-v
Copy link
Author

zorn-v commented Dec 24, 2020

I update deps (cordova to 10 and cordova-electron to 2) in https://github.com/zorn-v/cordova-electron-file-test for test and all works fine.

#371 (comment)

@Qualphey
Copy link

Yeah, the test does work! Thank you and happy holidays! ;)

@pjoriginal
Copy link

Any update on this?

@zorn-v
Copy link
Author

zorn-v commented Apr 21, 2021

I think never. Just use my branch

@RodriSanchez1
Copy link

So I've managed to add the new version of your plugin, but unfortunately it is still dumping Error: exec proxy not found for :: File :: requestFileSystem.

I have the same problem. How can i fix that @zorn-v ?

@zorn-v
Copy link
Author

zorn-v commented Jun 10, 2021

I have the same problem. How can i fix that @zorn-v ?

Just as I said before #371 (comment)

@eamanola
Copy link

eamanola commented Mar 30, 2022

Hi,

I'm getting
FileProxy.js:26 Electron Node.js integration is disabled, you can not use cordova-file-plugin without it...

with electron config:

{
  "browserWindow": {
    "width": 1536,
    "height": 864,
    "webPreferences": {
      "nodeIntegration": true
    }
  }
}

Tried #371 (comment) and rm -rf ~/.config/Electron. same message, only width height changes.

Any ideas?

platform: linux
cordova: 11.0
cordova-electron: 3.0.0
cordova-plugin-file: github:zorn-v/cordova-plugin-file#electron

@zorn-v
Copy link
Author

zorn-v commented Mar 30, 2022

Any ideas?

Maybe config reads from another place, maybe electron does not accept disabled node integration (I doubt) any more.
Lazy to check all versions.

@eamanola
Copy link

not too familiar with cordova or electron, the config seems to be the correct one thou. width and height do work.
no worries, thanks anyway.

@zorn-v
Copy link
Author

zorn-v commented Apr 2, 2022

electron/electron#23506
Seems this is related, taking into account that cordova-electron: 3.0.0 use electron v14
https://cordova.apache.org/announcements/2021/09/06/cordova-electron-release-3.0.0.html

@erisu
Copy link
Member

erisu commented Apr 3, 2022

Might try and follow this PR as a reference to update your PR.

Or create a new PR that is dedicated to the Cordova-Electron 3.0 support. Might be better to separate so if there is anyone who wants to continue to fork your current file pr for the older releases of Cordova-Electron.

apache/cordova-plugin-device#135

In your PR, I noticed it enforced users to use nodeIntegration with a hookscript. The hookscript modified core template files which it should not, IMO. If the user uninstalled the plugin for any reason, then the modified core files is not reverted. They would have to reinstall the platform to revert those changes.

For a side question, why not use fs-extra instead of rimraf?

https://nodejs.libhunt.com/compare-rimraf-vs-node-fs-extra

fs-extra appears to be more popular and well maintained by the community and with faster release cycles.

Anyways, with the Cordova plugin support in Cordova-Electron 3.0, you can now make an npm package that will be installed during the plugin install. You can set dependencies to the subpackage. We no longer need to commit the third-party libraries into the repo, which is something we try to avoid. It uses context isolation as well.

@zorn-v
Copy link
Author

zorn-v commented Apr 15, 2022

Might try and follow this PR as a reference to update your PR.

Or create a new PR that is dedicated to the Cordova-Electron 3.0 support. Might be better to separate so if there is anyone who wants to continue to fork your current file pr for the older releases of Cordova-Electron.

Is some sense in it ? My PR here already about 2 years without merge )

@marek8623
Copy link

marek8623 commented Apr 5, 2023

@zorn-v @erisu
Hi guys, when do you plan to merge latest changes? Thanks

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.

Electron platform - 'Missing Command Error' on API calls
10 participants