maximegris / angular-electron

Ultra-fast bootstrapping with Angular and Electron :speedboat:
https://www.maximegris.fr/angular-electron/
MIT License
5.64k stars 1.38k forks source link

Add support for preload scripts? #752

Open TheJakey opened 1 year ago

TheJakey commented 1 year ago

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!

glani commented 1 year ago

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.

maximegris commented 1 year ago

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.

TheJakey commented 1 year ago

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.

faribauc commented 1 year ago

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. :)

maximegris commented 1 year ago

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.

Dcx199302 commented 4 months ago

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
],```