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

feat: Use production build of react in packaged app #329

Merged
merged 1 commit into from
Apr 18, 2020

Conversation

gmaclennan
Copy link
Member

Currently we are using the development build of react in the packaged
app. The development build is slower, see:
https://reactjs.org/docs/optimizing-performance.html

The fix is a little complicated: Normally you would build a bundle (including React) with process.env.NODE_ENV = 'production'. However, we use webpack with target='electron-renderer' and this loads all modules as externals e.g. they are required at runtime rather than included in a single bundle. The webpack.environmentPlugin() does not set NODE_ENV for runtime, it only replaces the variable in moduels included in the bundle (react isn't included, because it is 'external').

The hacky workaround is to patch in process.env.NODE_ENV = 'production' to the output bundle from webpack. A better long-term solution might be to bundle all code for electron, which might also result in a smaller build size and faster loading (lots of require calls has an overhead, especially on slow machines with spinning disk hard drives). This would require filtering out dependencies from the packaged app which are required in the bundle.

TL;DR this change should make the app more responsive (snappier™) and should work well enough for now.

Currently we are using the development build of react in the packaged
app. The development build is slower, see:
https://reactjs.org/docs/optimizing-performance.html
@okdistribute
Copy link
Contributor

okdistribute commented Apr 17, 2020

@gmaclennan Thanks for this PR! We want to keep the package-lock.json file though, unless I'm missing something. Was that a mistake? Could you add it back?

Copy link
Contributor

@okdistribute okdistribute left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want to keep package-lock.json here

@gmaclennan
Copy link
Member Author

@okdistribute it's still there, I see it in the "Files changed" tab. It does have quite a few changes. I'm not sure how to install an additional module without changing it.

@okdistribute
Copy link
Contributor

Sigh, package-lock.. why..

@okdistribute okdistribute merged commit 1d22cc1 into master Apr 18, 2020
@okdistribute okdistribute deleted the feat/production-build branch April 18, 2020 18:12
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

Successfully merging this pull request may close these issues.

2 participants