Hello everyone:wave:! I'm getting ready to move the 2.0 beta to RC (release candidate) status as it is long overdue, but I first want to make sure that there aren't any breaking changes requested. If you think that the 2.0 beta is ready for RC then please :+1: this issue. If not, please :-1: and leave a comment explaining why.
Specifically, I'd like feedback on the following major changes:
nodeIntegration.Thanks for all your feedback!
I think electron-builder should not have node integration disabled by default.
By messing with Electron properties it adds an unnecessary complication layer. All Electron settings should be in 1 place - in the main Electron process - so that when you need to change things like node integration you didn't have to waste time figuring out where's what and why does electron-builder docs conflict with Electron docs. And what if I need to disable it only for 1 renderer but not the other ones? Will it override webPreferences.nodeIntegration of every window? People will flood you and the Electron repo with questions asking why don't their modules work anymore.
Please don't add unnecessary complexity layers.
To talk to these points:
Another point of feedback that would be useful:
@nklayman I found a problem with reloading windows in production. I think it would be nice if v2.0 didn't have this problem.
Ctrl + RTo prevent this problem, you could add the following in the main:
mainWindow.webContents.on('did-fail-load', () => {
if (process.env.NODE_ENV === 'production') {
mainWindow.loadURL('app://./index.html')
}
})
I added it right above this line and it fixed the problem:
if (process.env.WEBPACK_DEV_SERVER_URL) {
...
@aleksey-hoffman and @rathboma thanks for your suggestions! Regarding nodeIntegration:
nodeIntegration can turn an XSS attack into full backdoor access to a computer. This is a major security risk, and I'd like to prevent it being enabled on apps that don't need itnodeIntegration has to be adjusted from the vue.config.js is because the build process changes depending on whether or not it is enabledHowever, you made some valid points regarding complexity and how many apps will want nodeIntegration enabled. I think a good option would be to add a prompt for nodeIntegration when you install the plugin. This will allow developers to easily choose to enable nodeIntegration when creating a new project should they want that. The default will be false because that is the electron default and the more secure option. Does this seem like a reasonable comprimise?
@aleksey-hoffman as for the bug you reported, does this only happen with the 2.0 beta and not the 1.x version?
@nklayman regarding nodeIntegration. If 1.x enables it by overriding default Electron config, couldn't you just remove that override completely in 2.x and let developers set it to true in the main process themselves, if they need to?
I don't quite understand why do you want to make it configurable from within vue.config rather than just removing the override you added in 1.x completely. You see what I mean?
As for the bug, it emerges from Vue router and present in any "single page application (SPA)". Electron only sees 1 real page - index.html, so when you change the route to let's say /settings or whatever, and reload the window, Electron will try to load that /settings page but there's no such page, there's no settings.html, it's just a route. So the fix I mentioned above will simply load the index page if the window fails to load. I just tested the fix and it fixed the white screen issue.
Also, thanks for maintaining and developing this repo, it simplified the development and packaging process for me, I'm building a modern file manager app based on it, which I'm gonna open source on GitHub in about a month.
@aleksey-hoffman The reason I can't just leave it to be configured in the background.js file is because if the webpack build process allows native modules then it won't work without nodeIntegration (even if native modules are not used), so the webpack script needs to know if nodeIntegration is enabled or not. Rather than run a regex on the background.js which is unreliable I just have it be set from the pluginOptions config.
If you use hash instead of history mode does the bug still occur? You can also use history mode for SPA and hash for electron automatically.
I'm glad this plugin was helpful, and what you're building sounds really cool. I've been thinking about adding a showcase of projects built with this plugin and yours would be a great fit once you open source it, give me a ping when you do if you are interested.
@nklayman hmm, yeah, I had mode: 'history'. I commented out the previous fix and changed the mode to the line you referenced, and it also fixes that "reloading white screen" issue:
mode: process.env.IS_ELECTRON ? 'hash' : 'history',
Though I'm not sure if changin mode is gonna break anything. Doesn't look like it did from the first glance, but I'm not sure yet.
Yeah, sure thing, I will let you know when I publish it. I've been working on that file manager app for like the past 2 years, it will have lots of amazing features like:
Test_file_1.txt you can simply type something like tesg 1 (with typos, wrong case, missing words, missing symbols, and missing extension) and it will find it almost instantly.Wait till you see, it's gonna be really good, it's about 90% done already 🙂
@nklayman I finally figured out how to solve the problem where you would get Not allowed to load local resource when you tried to display an image in a renderer window with webSecurity: true.
I posted a solution here
Since you are trying to make electron-builder more secure by default, and since most devs simply set webSecurity: false just to fix the problem with image loading, perhaps we should somehow incorporate this into electron-builder v2.0? Create a custom protocol for loading media and describe how it works in the docs? I've seen many questions on Github and Stackoverflow, asking how to load images without disabling webSecurity but never found a single proper solution until now.
Perhaps, add something like this in the main:
app.on('ready', async () => {
const mediaLoadProtocolName = `${appName}-load-media`
protocol.registerFileProtocol(mediaLoadProtocolName, (request, callback) => {
const url = request.url.replace(`${mediaLoadProtocolName}://`, '')
// Decode URL to prevent errors when loading filenames with UTF-8 chars or chars like "#"
const decodedUrl = decodeURIComponent(url)
try {
return callback(decodedUrl)
}
catch (error) {
console.error(error)
return callback(404)
}
})
...
And then add some instructions to the docs on how to use it in a renderer:
<img
:src="getSrc('C:/test.jpg')"
data-path="C:/test.jpg"
>
methods: {
getSrc (path) {
return `app-name-load-media://${path}`
}
}
This new way of configuring nodeIntegration is confusing. I'm working on an app that can open multiple renderer windows, some of which are from webpages on the internet (i.e. an auth provider). Can I have nodeIntegration disabled in those windows but enabled on others? Does the vue.config.js option even override the params I pass to those windows?
What I have understood by now is that the config option doesn't affect my app, just the build process, so if I have it set to false, I could not use nodeIntegration with any of my windows because webpack will fail, but it it's set to true, I _can_ use it, but it's doesn't override the nodeIntegration parameter I pass to new BrowserWindow. Is that correct?
@skourismanolis The nodeIntegration option in vue.config.js not only changes the build process but also sets process.env.ELECTRON_NODE_INTEGRATION, which is used to enable/disable nodeIntegration in the electron launch config. This prevents you from having to update the configuration in two places.
@aleksey-hoffman I've created #742 to collect security-related recommendations for a new section in the docs. Can you repost your suggestion there for better tracking? If possible, I'd appreciate a short description of why switching webSecurity off is a bad idea and maybe any other pitfalls you ran into trying to keep webSecurity enabled. Thanks for your help!
@skourismanolis The
nodeIntegrationoption invue.config.jsnot only changes the build process but also setsprocess.env.ELECTRON_NODE_INTEGRATION, which is used to enable/disablenodeIntegrationin the electron launch config. This prevents you from having to update the configuration in two places.
So, as long as I don't use the process.env.ELECTRON_NODE_INTEGRATION env variable when I spawn my windows, I can control the nodeIntegration manually right?
Yep, but just beware that you might get errors because of build process changes from the nodeIntegration setting in vue.config.js.
I've been using the latest Beta versions for some time now, even in production. Hadn't had any issues upgrading from 1.4 and using the latest Electron 8. For testing i prefer Cypress and it's integration into Electron is a different topic, so sadly no feedback on Spectron..
As I am using native modules I have to enable the node integration, but I think it's a good thing to be "more secure" by default.
So far no (v2 related) bugs or new issues showed up, so i would consider it stable. As the app is still in beta and pretty new, it only has a limited user audience from around 300 people, but all operating systems (Linux, macOS, Windows) are represented.
@nklayman Electron v9 is out. Perhaps electron-builder v2 should support it out of the box?
Though, keep in mind, they switched the default value of app.allowRendererProcessReuse to true in Electron v9. So you will get an error if your app uses any non-context-aware or non-NAPI modules, after updating Electron to v9.
Of course, you could add a temporary override to the main:
app.allowRendererProcessReuse = false
But they will deprecate this override in Electron v11, so maybe developers should do this by themselves, if they want to keep using non-context-aware modules in Electron 9+?
V2 already uses electron v9 by default, the latest version just hasn't been published yet.
@nklayman Got any estimate release date yet? 😬
Need a bit more clarity on migration. my current testing project uses v1. I installed with simple steps given in master/readme.md. firstly, it prompted 3 electron versions, 4.x 5.x and 6.x. I fixed it with yarn upgrade --latest.
Now for migrating to v2, the doc says replace nodeIntegration:true with nodeIntegration: process.env.ELECTRON_NODE_INTEGRATION
It's not clear how this will change plugin v1 to v2? If you can give clear steps how to get started with v2 I think I can test stability out.
also according to release I see that there are only two changes, testing flow and disabled nodeIntegration. Is that the full changelog? Have you given a thought on adding basic examples for preload, contextIsolation etc.
@Sparkenstein I updated the migration guide with instructions on updating the plugin version, I forgot to add that. I added docs on preload with full instructions as well.
v2.0 has been promoted to RC and is now the default version!
@aleksey-hoffman I think loading local resources works just fine using the app:// protocol which is included. It is also automatically used if you just use ./file.png. Can you try this and see if it works in place of the custom protocol you suggested? If so, I can simplify the docs by just suggesting to use the app:// protocol.
@nklayman It doesn't work for me. When I try to load a media file, e.g.:
<img :src="app://E:/file.png">
I get this error:
GET app://e/file.png net::ERR_UNKNOWN_URL_SCHEME
It's weird that it changes drive letter from E:/ to e/ on Windows. it doesn't do that in my custom protocol. Perhaps that's what causes the error?
I'm using v1.4.6. Did anything change in v2.0.0? I didn't try it yet.
@aleksey-hoffman What happens if you use a relative path? For me those worked but absolute paths didn't.
@nklayman I just put the image file in the app directory and tried loading it with app://test.png and app://./test.png still the same error:
GET app://./test.png/ net::ERR_UNKNOWN_URL_SCHEME
Is that app protocol supposed to work only in production? Because I'm testing it in development.
I don't think I've deleted any default electron-builder functions from the background.js file, it's still registers the protocol scheme and calls the createProtocol('app') function, and loads the index.html on that protocol. I'm probably doing something wrong, but I'm not sure why doesn't it work like my own custom protocol.
To load files in my app, I'm using the implementation I described here. Maybe that custom protocol is doing something different, I'm not sure.
Anyway, I think most Electron apps are using absolute paths to load media because user files are not located in the app directory, so I'm not sure how useful it is to only have the ability to load relative paths.
Okay, I've done some more research and I found out that using app:// works just fine for loading out of the public folder, but for loading absolute paths you need the custom local-resource protocol. I've updated the docs to make this more clear. Thanks for your help debugging @aleksey-hoffman. BTW app:// only works in production, in dev public files will served on the dev server and can be accessed with /image.png. process.env.BASE_URL will automatically be set to either app:// or / in prod and dev.
@nklayman good to know. By the way, are you planning to add support for Vue 3 when it comes out of the Beta?
I tried updating my app to Vue 3 myself by running vue add vue-next but it broke everything and I don't really have time to figure out how to make it work, still trying to finish the file manager app I told you about...
Yes, I'd love for this plugin to support vue 3. I'll take a look at it sometime this week probably.
I don't know, I'm installing it using vue ui. In my package.json the
reported version is rc5
Il giorno ven 4 dic 2020 alle ore 20:42 Aleksey Hoffman <
[email protected]> ha scritto:
@realrecordzLab https://github.com/realrecordzLab how are you using
2.0.0-rc.5 if the latest version is 2.0.0-rc.4?—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/nklayman/vue-cli-plugin-electron-builder/issues/733#issuecomment-738979834,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AMKZMUWNRP64QZ467JK5W7DSTE3Q7ANCNFSM4MTG6QAA
.
Most helpful comment
Yes, I'd love for this plugin to support vue 3. I'll take a look at it sometime this week probably.