🔧 lerna #82

frinyvonnick · 2017-11-03T08:30:25Z

Lerna have an dependencies hoisting feature that prevent downloading multiple times a package by moving node_modules at root top level of your monorepo. The problem is react-highlight doesn't support react 16 so it use an older version of react that get into conflict with hoc-react-loader react version so tests fail. There is a pull request waiting to be merged that brings support for react 16 in react-highlight.

@fabienjuif we have two possibilities:

We wait for the PR to be merged
We rewrite examples site from scratch

fabienjuif · 2017-11-03T08:33:00Z

I prefer the rewrite way.

fabienjuif · 2017-11-03T08:33:38Z

package.json

-    ]
-  }
+	"private": true,
+	"workspaces": [


We both need it in package.json AND in lerna.json ?

This isn't for the same thing. workspaces is a config for yarn not lerna.

Yeah I can see that, but I think this is sad that lerna doesn't look for workspaces field when it set to "useWorkspaces": true for instance.

Is there a way to set lerna config into the package.json ?

They don't mention it in documentation

fabienjuif · 2017-11-03T08:34:33Z

packages/hoc-react-loader-examples/package.json

@@ -48,7 +48,7 @@
    "webpack-dev-server": "~1.12.1"
  },
  "dependencies": {
-    "hoc-react-loader": "file:../",
+    "hoc-react-loader": "~6.1.1",


Here, will lerna detect that hoc-react-loader is in a relative package and do the link on its own ?

And what if I upgrade the hoc-react-loader package to 7.0.0 without publishing it ?
What will do lerna ? Still doing a link, or download hoc-react-loader from NPM registry ?

There is an option to force local linking regardless of package version. I guess without this option it will download it from NPM registry.

fabienjuif · 2017-11-03T08:34:49Z

packages/hoc-react-loader/package.json

+    "test": "jest --collectCoverageFrom=src/**/*.js --collectCoverageFrom=src/**/*.jsx",
+    "coveralls": "jest --coverage && cat ./coverage/lcov.info | coveralls",
+    "ci": "npm-run-all --parallel lint coveralls"
+


empty line :p

fabienjuif · 2017-11-03T08:35:36Z

packages/hoc-react-loader/package.json

+      "import/prefer-default-export": 0,
+      "import/no-unresolved": 0,
+      "react/forbid-prop-types": 0,
+      "import/no-extraneous-dependencies": ["error", {"devDependencies": ["**/*.spec.js"]}]


Nice I didn't know about the exclusion :)

fabienjuif · 2017-11-03T08:46:24Z

@frinyvonnick I change the base of the PR (see #83) and the title (so this PR can be squashed when it's ready to be)

fabienjuif · 2017-11-03T08:52:41Z

As you see on the circle-ci, now on all my project I add a ci script.
So I don't have to maintain a circleci file, or a jenkins one, or ...
I just have to maintain my package.json :)

So we should let the ci script, and use npm-run-all to run subscripts (lint / test) from root and then let lerna doing its job.

frinyvonnick · 2017-11-03T09:09:29Z

Sure i didn't finished the lerna part. I didn't handle publish either.

frinyvonnick · 2017-11-06T20:33:31Z

packages/hoc-react-loader-examples/package.json

@@ -11,6 +12,7 @@
    "start": "react-scripts start",
    "build": "react-scripts build",
    "test": "react-scripts test --env=jsdom",
-    "eject": "react-scripts eject"
+    "eject": "react-scripts eject",
+    "ci": "npm-run-all --parallel test"


I wait for this PR to be merged to add linting to ci script.

coveralls · 2017-11-07T07:37:32Z

Coverage remained the same at 98.462% when pulling 5aac199 on tech/lerna into bfab580 on refactoring-protected.

fabienjuif · 2017-11-07T08:21:21Z

package.json

-      "./misc/testSetup.js"
-    ]
-  }
+	"private": true,


Why do you use tab in this one ?
Maybe you should run a yarn add some-dep && yarn remove some-dep to reindent it better. :p

fabienjuif · 2017-11-07T08:22:44Z

package.json

+		"packages/*"
+	],
+	"scripts": {
+		"start": "lerna run start --stream --ignore hoc-react-loader",


Why don't you use scope instead of ignore ?

fabienjuif

For me exemple folder should be simple (and empty), we will init react (or other) in an other PR. You just have to init a package.json with scripts that do echos (to validate lerna)
README.md should stay in the root directory
LICENSE should stay in the root directory
root package.json should have a name ? (dunno)
root package.json should have a description ? (dunno)
root package.json should have contributors (dunno ?)
etc

frinyvonnick · 2017-11-07T12:19:59Z

@fabienjuif no the root package.json is private. It handle only technical issues. Maybe we should inspire ourselves from package.json like babel's one.

Done, i mentioned the remaining work in meta issue

fabienjuif

nice work 👌

frinyvonnick added 2 commits November 2, 2017 18:56

🎨 Move sources into packages folder

8b6a508

🚨 Link packages and fix linting

dcceecd

fabienjuif reviewed Nov 3, 2017

View reviewed changes

fabienjuif changed the base branch from master to refactoring-protected November 3, 2017 08:45

fabienjuif changed the title ~~Tech/lerna~~ 🔧 lerna Nov 3, 2017

frinyvonnick added 3 commits November 6, 2017 21:14

🔥 Remove examples site and fresh install one with create-react-app

3f4f59a

🔨 Remove empty line

5a0c394

💚 Fix ci script

0af860e

frinyvonnick commented Nov 6, 2017

View reviewed changes

frinyvonnick added 4 commits November 7, 2017 08:01

🔥 Remove default content

5991f78

🔧 Remove non-existent option on lerna bootstrap

34cc8c1

🔧 Add start script for examples in root package.json

84f744a

🔨 Set right package and version in hoc-react-loader-examples

5aac199

fabienjuif reviewed Nov 7, 2017

View reviewed changes

fabienjuif previously requested changes Nov 7, 2017

View reviewed changes

frinyvonnick added 3 commits November 7, 2017 13:24

🔨 Remove tab characters

46a657d

👌 Use scope instead of ignore option

ed4fbe9

🎨 Move LICENSE and README.md to root level

2b7b1ca

frinyvonnick requested a review from fabienjuif November 7, 2017 13:32

fabienjuif approved these changes Nov 7, 2017

View reviewed changes

frinyvonnick merged commit 958b1ec into refactoring-protected Nov 7, 2017

frinyvonnick deleted the tech/lerna branch November 7, 2017 14:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🔧 lerna #82

🔧 lerna #82

frinyvonnick commented Nov 3, 2017

fabienjuif commented Nov 3, 2017

fabienjuif Nov 3, 2017

frinyvonnick Nov 3, 2017

fabienjuif Nov 3, 2017

frinyvonnick Nov 6, 2017

fabienjuif Nov 3, 2017

frinyvonnick Nov 3, 2017

fabienjuif Nov 3, 2017

frinyvonnick Nov 6, 2017

fabienjuif Nov 3, 2017

fabienjuif Nov 3, 2017

fabienjuif commented Nov 3, 2017

fabienjuif commented Nov 3, 2017

frinyvonnick commented Nov 3, 2017 •

edited

Loading

frinyvonnick Nov 6, 2017

coveralls commented Nov 7, 2017

fabienjuif Nov 7, 2017 •

edited

Loading

fabienjuif Nov 7, 2017

fabienjuif left a comment

frinyvonnick commented Nov 7, 2017 •

edited

Loading

fabienjuif left a comment

🔧 lerna #82

🔧 lerna #82

Conversation

frinyvonnick commented Nov 3, 2017

fabienjuif commented Nov 3, 2017

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabienjuif commented Nov 3, 2017

fabienjuif commented Nov 3, 2017

frinyvonnick commented Nov 3, 2017 • edited Loading

Choose a reason for hiding this comment

coveralls commented Nov 7, 2017

fabienjuif Nov 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabienjuif left a comment

Choose a reason for hiding this comment

frinyvonnick commented Nov 7, 2017 • edited Loading

fabienjuif left a comment

Choose a reason for hiding this comment

frinyvonnick commented Nov 3, 2017 •

edited

Loading

fabienjuif Nov 7, 2017 •

edited

Loading

frinyvonnick commented Nov 7, 2017 •

edited

Loading