Skip to content
This repository has been archived by the owner on Oct 1, 2020. It is now read-only.

Why electron-compile is required for production #207

Open
develar opened this issue Mar 20, 2017 · 13 comments
Open

Why electron-compile is required for production #207

develar opened this issue Mar 20, 2017 · 13 comments

Comments

@develar
Copy link
Member

develar commented Mar 20, 2017

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

sidneys commented Mar 23, 2017

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

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

sidneys commented Mar 23, 2017

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

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

develar commented Mar 23, 2017

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

develar commented Mar 23, 2017

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

develar commented Mar 23, 2017

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

develar commented Mar 23, 2017

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." (electron-userland/electron-builder#807 (comment)).

@anaisbetts
Copy link
Contributor

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

develar commented Mar 23, 2017

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

develar commented Mar 23, 2017

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

@develar
Copy link
Member Author

develar commented Mar 24, 2017

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 #209

@Hammster
Copy link

Hammster commented Jan 25, 2018

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 ?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants