electron-userland / electron-compile

DEPRECATED: Electron supporting package to compile JS and CSS in Electron applications
1.01k stars 97 forks source link

Why electron-compile is required for production #207

Open develar opened 7 years ago

develar commented 7 years ago

Hi. I supported electron-compile in the electron-builder, but what I don't understand it is why we need es6-shim.js and electron-compile dependency in the runtime.

If all files were transpiled, for what we need to use shim?

Also, electron-compile has some deps that are not required for production — e.g. yargs / spawn-rx.

sidneys commented 7 years ago

@develar Have the same problem. Moving electron-compile to devDependencies as recommended in the electron-builder warning leads to es6-shim.js not finding electron-compile on runtime of build production apps, hence crashing the app.

If there is a comprehensive approach on converting or moving electron-compile-based apps – which almost always have an es6-init.js, see README – to comply with electron-builder (and get rid of the new warning), a small how-to would be great.

MarshallOfSound commented 7 years ago

@develar es6-shim is used to set up Electron Compile for the user, making the user do this themselves is unnecessary as it will be the same setup steps for each application. It makes more sense for us to do the meaningless task ourselves 👍

electron-compile is needed as a production dependency as files aren't transpiled in the traditional "in-place" sense, they are transpiled to a cache which is then loaded and read by electron-compile at run time to reduce the load time of transpiled files. Everything is in this cache 👍 @paulcbetts Might be able to explain a bit more about how this works

As for the unused dependencies, yargs is pretty lightweight I think, and I'm pretty sure we can remove spawn-rx as the recommended way of using electron-compile is with electron-forge so we don't need an internal packager CLI anymore.

@sidneys

Moving electron-compile to devDependencies as recommended in the electron-builder warning

This warning is just straight up wrong, electron-compile is a production dependency

sidneys commented 7 years ago

@MarshallOfSound @develar

Then there seems to be a conflict here, as electron-builder states

⚠️  Package "electron-compile" should be in "devDependencies".
Please remove it from the "dependencies" section in your package.json.
Please see https://github.com/electron/electron-compile/issues/207

However, following this approach results in production builds missing electron-compile..

MarshallOfSound commented 7 years ago

@sidneys Not a conflict, a misunderstanding of the requirements of -compile. It is a prod dependency and needs to be in the "dependencies" section

develar commented 7 years ago

@MarshallOfSound for what we need electron-compile and shim if all files were transpiled? Electron-builder uses electron-compile to compile all files in place.

develar commented 7 years ago

@sidneys readme is clear — shim is required only if you select ”How does it work? (Slightly Harder Way“

If you will you use easy way — special electron prebuilt with electron-complie, shim not required in both dev and production modes.

I don't see any reason why electron-compile is required for production — so, I have opened this ticket to clear it.

develar commented 7 years ago

to reduce the load time of transpiled files

Yes, I read sources, compiler just throw error if no file in the cache. But it doesn't related to IO performance since asar is used. It just adds unnecessary dependency and checks.

develar commented 7 years ago

Short summary from private talk with @MarshallOfSound to avoid misunderstandings — electron-builder doesn't attempt to support electron-compile in a different way or somehow contradicts to. Currently it is implemented in this way (and warn printed) only because original question in this issue is not answered yet.

Nothing more.

electron-builder just want to provide the best experience for users and have the answer to question like "I had exactly the same problem where this shim was used in production build. I switched to pure babel pre-compilation since then and never looked back." (https://github.com/electron-userland/electron-builder/issues/807#issuecomment-287743382).

anaisbetts commented 7 years ago

I don't see any reason why electron-compile is required for production — so, I have opened this ticket to clear it.

@develar While I guess technically you're correct, you could use electron-compiler to just pre-compile everything in-place, electron-compile in production is actually less I/O than pre-compiling because electron-compile GZips everything in its cache. It also prevents your app from loading file:// content you didn't precompile, which is an important security mitigation.

That being said, I guess there's nothing stopping you from doing so, though I think you'll have a better experience if you use electron-compile as-designed

develar commented 7 years ago

is actually less I/O than pre-compiling because electron-compile GZips everything in its cache

because CPU to ungzip is cheeper than read data from disk (asar file)? Interesting, thanks.

It also prevents your app from loading file:// content you didn't precompile, which is an important security mitigation

👍

Thanks for answer. Now all is clear.

develar commented 7 years ago

@paulcbetts @MarshallOfSound Another question — why source directory is still included? src? Is it required for something or just because not important to ignore?

develar commented 7 years ago

As for the unused dependencies, yargs is pretty lightweight I think, and I'm pretty sure we can remove spawn-rx as the recommended way of using electron-compile is with electron-forge so we don't need an internal packager CLI anymore.

👍 I have filed https://github.com/electron/electron-compile/issues/209

Hammster commented 6 years ago

I'm facing the same question as @develar even though from another viewpoint , do i really need debug rimraf spawn-rx mkdirp packages in a release build?

The project which i'm working on currently has 3 packages (vue, vuex, electon-edge-js,) besides the 68 that come with electron-compile, could there not be a electron-compile-essential to at least lower the amount of external packages that i have to introduce in any project that uses electron-compile ?