WIP: Relates to #81. Update to Electron v3.1.0 #90

ltfschoen · 2019-01-09T03:15:07Z

Update to Electron v3.1.0 - https://github.com/electron/electron/releases
Update Readme
Update dependencies versions
Add .nvmrc

* Update to Electron v3.1.0 - https://github.com/electron/electron/releases * Update Readme * Update dependencies versions * Add .nvmrc

Otherwise when try to use the package from a remote branch such as ``` "@parity/electron": "https://github.com/paritytech/js-libs.git#luke-81-electron-3", ``` Then it produces error: ``` error Can't add "@parity/electron": invalid package version undefined. ```

When I add this to the fether-electron package.json dependency with: ``` "@parity/electron": "https://github.com/paritytech/js-libs.git#luke-81-electron-3", ``` And then try to run `yarn; yarn build;`, it gives the following error: ``` scon @ ~/code/src/paritytech/fether - [luke-309-electron-3] $ yarn; yarn build yarn install v1.12.3 [1/5] 🔍 Validating package.json... [2/5] 🔍 Resolving packages... [3/5] 🚚 Fetching packages... [4/5] 🔗 Linking dependencies... warning " > babel-eslint@10.0.1" has unmet peer dependency "eslint@>= 4.12.1". warning " > fether-ui@0.2.0" has unmet peer dependency "prop-types@^15.6.1". warning " > fether-ui@0.2.0" has unmet peer dependency "react@^16.4.0". warning "workspace-aggregator-cc67e372-3487-4efc-8fcb-a403fb15962c > fether-react > @parity/light.js@3.0.11" has incorrect peer dependency "rxjs@~6.2.2". warning "workspace-aggregator-cc67e372-3487-4efc-8fcb-a403fb15962c > fether-react > @parity/light.js-react@3.0.11" has incorrect peer dependency "rxjs@~6.2.2". warning "workspace-aggregator-cc67e372-3487-4efc-8fcb-a403fb15962c > fether-react > react-final-form@3.6.4" has unmet peer dependency "prop-types@^15.6.0". warning "workspace-aggregator-cc67e372-3487-4efc-8fcb-a403fb15962c > fether-react > react-final-form-listeners@1.0.1" has unmet peer dependency "prop-types@^15.6.0". warning "workspace-aggregator-cc67e372-3487-4efc-8fcb-a403fb15962c > fether-react > @babel/plugin-proposal-decorators@7.2.0" has unmet peer dependency "@babel/core@^7.0.0-0". warning "workspace-aggregator-cc67e372-3487-4efc-8fcb-a403fb15962c > fether-ui > semantic-ui-react@0.81.3" has unmet peer dependency "react-dom@>=0.14.0 <= 16". warning "workspace-aggregator-cc67e372-3487-4efc-8fcb-a403fb15962c > fether-react > @babel/plugin-proposal-decorators > @babel/plugin-syntax-decorators@7.2.0" has unmet peer dependency "@babel/core@^7.0.0-0". [5/5] 📃 Building fresh packages... [1/2] ⠄ @parity/electron error /Users/scon/code/src/paritytech/fether/node_modules/@parity/electron: Command failed. Exit code: 1 Command: yarn build Arguments: Directory: /Users/scon/code/src/paritytech/fether/node_modules/@parity/electron Output: yarn run v1.12.3 $ lerna exec yarn build --stream lerna notice cli v3.10.1 lerna info Executing command in 6 packages: "yarn build" @parity/abi: $ rimraf lib @parity/electron: $ rimraf lib @parity/abi: $ tsc @parity/abi: /bin/sh: tsc: command not found @parity/electron: $ tsc @parity/abi: error Command failed with exit code 127. @parity/abi: info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command. @parity/electron: /bin/sh: tsc: command not found lerna ERR! yarn build exited 1 in '@parity/abi' lerna WARN complete Waiting for 1 child process to exit. CTRL-C to exit immediately. error Command failed with exit code 1. info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command. ```

amaury1093

Thanks luke! Though I have questions.

Also, you added the help wanted label, do you have some questions?

amaury1093 · 2019-01-09T09:26:48Z

.nvmrc

@@ -0,0 +1 @@
+10.11.0


Why is this needed actually? Is there a package we use that require at least this version of node? Is it redundant with what you added in package.json?

In the Electron Release Notes for v4.0.0 it says:

Bumped minimum supported macOS version to 10.10.

And on this page they mention no more support for 10.9 in Electron v4 https://electronjs.org/blog/electron-4-0#no-more-macos-109-support

So I've assumed that v3.1.0 would have the same requirement. However the release notes for v4.0.1 or v3.1.0 don't mention a minimum version requirement https://github.com/electron/electron/releases

We could change it to 10.10.0 instead

amaury1093 · 2019-01-09T09:29:27Z

README.md

+#### Tests
+
+```
+export NPM_TOKEN=''; yarn test


No need for export NPM_TOKEN='';. Can you confirm @ltfschoen?

Yes confirmed. It is no longer required

I just created another issue. It looks like this issue came back again

amaury1093 · 2019-01-09T09:29:39Z

README.md

+#### Build
+
+```
+export NPM_TOKEN=''; yarn build


No need for export NPM_TOKEN='';

amaury1093 · 2019-01-09T09:30:21Z

README.md

+5. Run tests, linting, and build
+
+```
+export NPM_TOKEN=''; yarn test; yarn lint; yarn build


No need for export NPM_TOKEN='';

amaury1093 · 2019-01-09T09:33:21Z

package.json

@@ -1,6 +1,7 @@
 {
  "name": "js-libs",
  "description": "A collection of JavaScript libraries for dapp development.",
+  "version": "v3.0.26",


No need to have version on the root package.json, see for example babel

If I pushed my changes in this branch 'luke-81-electron-3' of js-libs repo without a version number in its package.json, then when I tried to test using this remote branch in a branch of Fether 'luke-309-electron-3' on this line of code https://github.com/paritytech/fether/pull/349/files#diff-52ba44750a2113719866b7d98b31af30R42 "@parity/electron": "https://github.com/paritytech/js-libs.git#luke-81-electron-3",, I found that when I tried to build Fether with yarn; yarn start it would give me an error that it required a version number in the package.json file

It produced the following:

error Can't add "@parity/electron": invalid package version undefined.

In the details of the commit message I included the full log 8b347e0

amaury1093 · 2019-01-09T09:35:03Z

packages/abi/package.json

@@ -17,11 +17,12 @@
  "scripts": {
    "docs": "typedoc",
    "prebuild": "rimraf lib",
-    "build": "tsc",
+    "build": "yarn && ./node_modules/.bin/tsc",


On my machine there's no need for this addition. Did you get an error?

Yes, I was getting the following errors:

@parity/abi: /bin/sh: tsc: command not found @parity/electron: /bin/sh: tsc: command not found

In the details of the commit message I included the full log 73c2042

These errors are in the fether, right? If yes, then see my comment about removing step 7

amaury1093 · 2019-01-09T09:39:23Z

packages/electron/package.json

  },
  "devDependencies": {
    "@types/async-retry": "^1.2.1",
    "@types/checksum": "^0.1.30",
-    "electron": "^2.0.2"
+    "electron": "^3.1.0",


I want this package to be available for all electron versions. The only weak link is electron-dl, which I am not 100% sure is compatible with Electron 3-4, but let's assume yes until proven wrong (and do some testing on Fether). All other packages are only nodejs.

So replace with "^2.0.2 | ^3.1.0 | ^4.0.1"

If I also update Fether's fether-electron package's package.json with:

"devDependencies": { ... "electron": "^2.0.2 | ^3.1.0 | ^4.0.1",

Then when I run yarn; yarn start it prompts me to choose the version, but it shows versions prior as well like 1.x.x

? Please choose a version of "electron" from this list: (Use arrow keys) ❯ 4.0.1 4.0.0 4.0.0-nightly.20181010 ... 4.0.0-beta.1 3.1.0 3.1.0-beta.5 ... 3.0.4 (Move up and down to reveal more choices)

In js-libs, we put "^2.0.2 | ^3.1.0 | ^4.0.1", which means "this library supports these versions of Electron."

In Fether, we choose one specific version of Electron (4.0.1). So in fether it should only be "^4.0.1"

amaury1093 · 2019-01-09T09:39:29Z

packages/electron/package.json

  },
  "peerDependencies": {
-    "electron": "^2.0.3"
+    "electron": "^3.1.0"


"^2.0.2 | ^3.1.0 | ^4.0.1"

packages/light.js-react/package.json

packages/light.js/package.json

amaury1093 · 2019-01-09T09:48:21Z

README.md

+3. Check outdated dependencies
+
+```
+npm outdated


yarn outdated

amaury1093 · 2019-01-09T09:49:48Z

README.md

+npm outdated
+```
+
+4. Create a branch and update any dependencies that it indicates are out of date.


I'd prefer that it'd be us who update dependencies. They sometimes make things break. Or even better: implement #28.

amaury1093 · 2019-01-11T09:38:52Z

README.md

+
+6. Push the branch to your fork of the repo
+
+7. Integrate the updated library as a dependency. Example: If you are using the 'electron' package in another project, then update the package.json file to temporarily use your branch instead of the public NPM registry version


@ltfschoen I understand now where your errors come from. I would like to remove this step 7: For @parity/* packages, I would advise against doing yarn add github:something into your own project.

The reason is:

right now we develop in src/ in js-libs

the CI builds, outputs in lib/, and pushes lib/ to npm

when you yarn add @parity/* it downloads lib/ from npm into node_modules, and the main file is lib/index.js

However, when you yarn add github:something, it does:

clone the repo into node_modules

run yarn build inside the cloned folder

hopefully, if the build doesn't error, it generates a lib/ folder

Jaco and I used to try to do this method above, however there's always some troubles here and there in the "hopefully" step. That's why I prefer to remove it. Some alternative solutions:

(ideal solution) wait for the npm package to be published. If you need to test a package, it's better to add a test directly in this repo, so that the test is self-contained.

use npm link

do the copy/paste dance:

# in js-libs/ yarn build cp -r packages/my-package/lib ../fether/node_modules/@parity/my-package/lib # in fether yarn start

amaury1093 · 2019-01-11T09:40:21Z

.nvmrc

@@ -0,0 +1 @@
+10.10.0


My questions are:

If your nvm has v11, does it change to v10.10.0? (not necessary)

Is it enough to only put "engine" in package.json?

You're right. I'll remove the .nvmrc file. It's more useless than I thought

If I'm using the wrong Node.js version it warns me when I run yarn as follows:

scon @ ~/code/src/paritytech/js-libs - [luke-81-electron-4] $ nvm use 10.6.0 Now using node v10.6.0 (npm v6.1.0) scon @ ~/code/src/paritytech/js-libs - [luke-81-electron-4] $ yarn yarn install v1.12.3 [1/5] 🔍 Validating package.json... error js-libs@3.0.26: The engine "node" is incompatible with this module. Expected version ">=10.10.0". Got "10.6.0" error Found incompatible module info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.

You're right, there's just as many steps whether we do or don't have the .nvmrc file. The important thing is to have the "engine" specified.

scon @ ~/code/src/paritytech/js-libs - [luke-81-electron-4] $ node --version v10.6.0 scon @ ~/code/src/paritytech/js-libs - [luke-81-electron-4] $ nvm use No .nvmrc file found

With .nvmrc file:

scon @ ~/code/src/paritytech/js-libs - [luke-81-electron-4] $ node --version v10.6.0 scon @ ~/code/src/paritytech/js-libs - [luke-81-electron-4] $ nvm use Found '/Users/scon/code/src/paritytech/js-libs/.nvmrc' with version <10.10.0> N/A: version "10.10.0 -> N/A" is not yet installed. You need to run "nvm install 10.10.0" to install it before using it. scon @ ~/code/src/paritytech/js-libs - [luke-81-electron-4] $

ltfschoen · 2019-01-13T02:42:45Z

Closing as replaced by PR #91 and comments being incorporated.

See openethereum/js-libs#90 (comment)

ltfschoen added 3 commits January 9, 2019 14:12

feat: Relates to #81. Update to Electron v3.1.0

1bd8d8f

* Update to Electron v3.1.0 - https://github.com/electron/electron/releases * Update Readme * Update dependencies versions * Add .nvmrc

ltfschoen added the help wanted Extra attention is needed label Jan 9, 2019

ltfschoen changed the title ~~feat: Relates to #81. Update to Electron v3.1.0~~ WIP: Relates to #81. Update to Electron v3.1.0 Jan 9, 2019

ltfschoen added bug Something isn't working WIP labels Jan 9, 2019

ltfschoen mentioned this pull request Jan 9, 2019

feat: Relates to #312. Taskbar app openethereum/fether#332

Merged

23 tasks

amaury1093 suggested changes Jan 9, 2019

View reviewed changes

amaury1093 reviewed Jan 9, 2019

View reviewed changes

README.md Outdated

3. Check outdated dependencies

```

npm outdated

Copy link

Collaborator

amaury1093 Jan 9, 2019

Choose a reason for hiding this comment

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

yarn outdated

ltfschoen reacted with thumbs up emoji

amaury1093 reviewed Jan 9, 2019

View reviewed changes

review-fix: Updates based on review comments so far

2e2d809

ltfschoen mentioned this pull request Jan 10, 2019

WIP: Relates to #81. Update to Electron v4.1.0 #91

Merged

amaury1093 reviewed Jan 11, 2019

View reviewed changes

ltfschoen closed this Jan 13, 2019

ltfschoen added a commit to openethereum/fether that referenced this pull request Jan 13, 2019

review-fix: Only specify required version.

10093dd

See openethereum/js-libs#90 (comment)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Relates to #81. Update to Electron v3.1.0 #90

WIP: Relates to #81. Update to Electron v3.1.0 #90

ltfschoen commented Jan 9, 2019

amaury1093 left a comment

amaury1093 Jan 9, 2019

ltfschoen Jan 10, 2019 •

edited

amaury1093 Jan 9, 2019

ltfschoen Jan 10, 2019

ltfschoen Jan 13, 2019

amaury1093 Jan 9, 2019

amaury1093 Jan 9, 2019

amaury1093 Jan 9, 2019

ltfschoen Jan 10, 2019 •

edited

ltfschoen Jan 10, 2019

amaury1093 Jan 9, 2019

ltfschoen Jan 10, 2019 •

edited

amaury1093 Jan 11, 2019

amaury1093 Jan 9, 2019

ltfschoen Jan 10, 2019

amaury1093 Jan 11, 2019

amaury1093 Jan 9, 2019

amaury1093 Jan 9, 2019

amaury1093 Jan 9, 2019

amaury1093 Jan 11, 2019 •

edited

amaury1093 Jan 11, 2019

ltfschoen Jan 13, 2019

ltfschoen commented Jan 13, 2019


		6. Push the branch to your fork of the repo

		7. Integrate the updated library as a dependency. Example: If you are using the 'electron' package in another project, then update the package.json file to temporarily use your branch instead of the public NPM registry version

		@@ -0,0 +1 @@
		10.11.0

		@@ -0,0 +1 @@
		10.10.0

WIP: Relates to #81. Update to Electron v3.1.0 #90

WIP: Relates to #81. Update to Electron v3.1.0 #90

Conversation

ltfschoen commented Jan 9, 2019

amaury1093 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ltfschoen Jan 10, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ltfschoen Jan 10, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ltfschoen Jan 10, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amaury1093 Jan 11, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ltfschoen commented Jan 13, 2019

ltfschoen Jan 10, 2019 •

edited

ltfschoen Jan 10, 2019 •

edited

ltfschoen Jan 10, 2019 •

edited

amaury1093 Jan 11, 2019 •

edited