Support Hot Module Replacement #824

2017-12-07T17:53:30+01:00

IGassmann commented

2017-12-07 17:53:30 +01:00

(Migrated from github.com)

This pull request contains essentially:

Convert project into a one package.json file structure
Use electron-webpack for development
Support HMR (hot reloading)
Update compilation and building tools (see README.md)

This pull request contains essentially: - Convert project into a one package.json file structure - Use [electron-webpack](https://github.com/electron-userland/electron-webpack) for development - Support HMR (hot reloading) - Update compilation and building tools (see README.md)

alexliebowitz (Migrated from github.com) reviewed 2017-12-07 17:53:30 +01:00

neb-b (Migrated from github.com) reviewed 2017-12-07 17:53:30 +01:00

liamcardenas (Migrated from github.com) requested changes 2017-12-07 19:05:45 +01:00

liamcardenas (Migrated from github.com) left a comment

Overall looks great, but these changes are very dense. I will have to clone this and play around with it.

src/renderer/jsonrpc.js

					
				@ -0,0 +17,4 @@

				        let error;

				        if (json.error) {

				          error = new Error(json.error);

				        } else {

liamcardenas (Migrated from github.com) commented

2017-12-07 19:00:21 +01:00

console.error(e)

src/renderer/webpack.dev.js

liamcardenas (Migrated from github.com) commented

2017-12-07 19:03:37 +01:00

is flow working with this webpack config?

kauffj (Migrated from github.com) reviewed 2017-12-07 19:27:29 +01:00

kauffj (Migrated from github.com) left a comment

This looks great!

README.md

kauffj (Migrated from github.com) commented

2017-12-07 19:23:35 +01:00

losing

package.json

					
				@ -68,2 +7,4 @@

				    "email": "hello@lbry.io"

				  },

				  "homepage": "https://lbry.io/",

				  "scripts": {

kauffj (Migrated from github.com) commented

2017-12-07 19:24:31 +01:00

Where did this line go? Does it not do anything?

IGassmann (Migrated from github.com) reviewed 2017-12-07 19:34:29 +01:00

package.json

					
				@ -68,2 +7,4 @@

				    "email": "hello@lbry.io"

				  },

				  "homepage": "https://lbry.io/",

				  "scripts": {

IGassmann (Migrated from github.com) commented

2017-12-07 19:34:29 +01:00

The electron-builder configuration has been moved into its own file.

I removed the category line since the app isn't really a utility compared to the Mac apps categorized as utilities on the App Store. Maybe the category "video" would be more appropriate? https://developer.apple.com/library/content/documentation/General/Reference/InfoPlistKeyReference/Articles/LaunchServicesKeys.html#//apple_ref/doc/uid/TP40009250-SW8

The `electron-builder` configuration has been moved into its [own file](https://github.com/lbryio/lbry-app/blob/restructuring/electron-builder.json). I removed the `category` line since the app isn't really a utility compared to the Mac apps categorized as utilities on the App Store. Maybe the category "video" would be more appropriate? https://developer.apple.com/library/content/documentation/General/Reference/InfoPlistKeyReference/Articles/LaunchServicesKeys.html#//apple_ref/doc/uid/TP40009250-SW8

IGassmann (Migrated from github.com) reviewed 2017-12-07 21:06:38 +01:00

src/renderer/webpack.dev.js

IGassmann (Migrated from github.com) commented

2017-12-07 21:06:38 +01:00

Now it does.

kauffj (Migrated from github.com) reviewed 2017-12-07 23:48:47 +01:00

package.json

					
				@ -68,2 +7,4 @@

				    "email": "hello@lbry.io"

				  },

				  "homepage": "https://lbry.io/",

				  "scripts": {

kauffj (Migrated from github.com) commented

2017-12-07 23:48:46 +01:00

Please retain this setting. If you want to change it to video or entertainment I am fine with that.

liamcardenas (Migrated from github.com) approved these changes 2017-12-08 18:50:37 +01:00

liamcardenas (Migrated from github.com) left a comment

I am happy with this, we squashed every bug we could find and verified that key functionality still worked. It's impossible to check everything (this is one of those times that I wish we did more unit testing) so I say lets merge it and hope for the best / fix whatever comes up.

Sign in to join this conversation.

No reviewers

neb-b

alexliebowitz

liamcardenas