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

Add support for preload scripts?

Open TheJakey opened this issue 2 years ago • 6 comments

Hello, I don't quite understand, why is there no Preload script in here since it's recommended practice?

Also nodeIntegration: true is based on several high-upvoted posts on StackOverflow just a huge security risk e.g. https://stackoverflow.com/a/59888788/12139920

Thanks for ur insights!

TheJakey avatar Apr 09 '23 18:04 TheJakey

No problems with preloaded scripts and node integration true if you are not going to implement your own browser and load URLs from the untrusted world. For the sandboxed application it is ok. Yes, you should understand the dependencies you use.

glani avatar Apr 16 '23 21:04 glani

Hi @TheJakey,

It's a dilemma I've always had on this project. 😅 Between the ease of handling it or implenting more advanced features with nodeIntegration and contextIsolation. I chose to keep it simple and let everyone the possibility to adapt depending of his/her project or business case.

Btw I should add a warning about that in the README & links to Electron documentation to let people know.

maximegris avatar Apr 21 '23 18:04 maximegris

Well, I'm currently struggling with packaging the app (I decided to go with preload and context isolation) and damn it's a hustle to make it work (due to some dependencies calling require(), among other issues). >_<

So I totally see that this approach is much easier to handle.

It sounds like a great idea to add some explanation to README about this topic.

TheJakey avatar Apr 21 '23 18:04 TheJakey

Here's a quick fork I made today that adds support for preloads:

https://github.com/faribauc/angular-electron

See specific changes here: https://github.com/maximegris/angular-electron/commit/5ae0b86a3ce06e7106db9c1fdfe25473839b39d6

Please go ahead and comment over there with any suggestions you may have!

@maximegris Let me know if this would be worth a PR for your repo. :)

faribauc avatar May 11 '23 19:05 faribauc

Hi @faribauc Thank you for spending time on this. :)

I think there are some changes to do in the electron.service.ts too. Now isElectron() always return false because the test must be on window.electronAPI.

I guess conditional imports in electron.service.ts constructor have to move in preloads too. Or instead have a chapter about how preloads work in README. Because those are not relevant or useful other than to ensure that everything is working properly.

maximegris avatar May 14 '23 08:05 maximegris

This is my way of adding preloaded scripts, and it works fine so far.

new BrowserWindow({
    webPreferences: {
      defaultEncoding: 'UTF-8',
      nodeIntegration: false, 
      contextIsolation: true,
      preload: path.join(__dirname, './preload.js'),
    },
});

// tsconfig.server.json
"files": [
    "app/main.ts",
    "app/preload.ts" // add
],```

Dcx199302 avatar May 04 '24 05:05 Dcx199302