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

Add support for preload scripts? #752

Open
TheJakey opened this issue Apr 9, 2023 · 6 comments
Open

Add support for preload scripts? #752

TheJakey opened this issue Apr 9, 2023 · 6 comments

Comments

@TheJakey
Copy link

TheJakey commented Apr 9, 2023

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
Copy link

glani commented Apr 16, 2023

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
Copy link
Owner

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
Copy link
Author

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
Copy link

faribauc commented May 11, 2023

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

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

See specific changes here: 5ae0b86

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
Copy link
Owner

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
Copy link

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

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

No branches or pull requests

5 participants