aurelia / webpack-plugin

A plugin for webpack that enables bundling Aurelia applications.
MIT License
90 stars 36 forks source link

fix(getPAL): use browser-pal for electron-renderer #100

Closed MeirionHughes closed 7 years ago

niieani commented 7 years ago

It might make sense to also use urelia-pal-nodejs for electron renderer (it should also work).

jods4 commented 7 years ago

I was suggested as reviewer but honestly guys I have no idea about electron stuff. I'm wondering if aurelia-pal-browser shouldn't be the default, though....

I'm not using Electron at all, so if someone with more experience tells me it's the good choice, then let's merge.

MeirionHughes commented 7 years ago

the renderer thread is chromium; I don't see why you'd want to use pal-nodejs within what is effectively chrome. A good reason for using pal-browser is you get full SVG support.

MeirionHughes commented 7 years ago

here is working repo btw: https://github.com/MeirionHughes/aurelia-webpack-electron its working flawlessly so far. I noticed you can override the pal via the opts (better late than never).

niieani commented 7 years ago

@MeirionHughes

I don't see why you'd want to use pal-nodejs within what is effectively chrome

You can use pal-nodejs in Electron to effectively skip any build/compilation steps. Since Electron is Node + Chromium, you get the best of both worlds. This way you can use Aurelia from pure JavaScript or pure TypeScript (if you use something like ts-node/register or even ESNext when using babel-register). NodeJS effectively becomes the loader and no bundling is necessary. Of course, HMR won't work this way at the moment (it would need a plugin which watches files for changes), but it should be possible in the future.

MeirionHughes commented 7 years ago

You can use pal-nodejs in Electron to effectively skip any build/compilation steps.

would that not imply you are not using this webpack plugin?

niieani commented 7 years ago

@MeirionHughes ah, yes! Forgot we're in the webpack-plugin context. So in that case, this PR totally makes sense. Let's merge it!