Lerna workspace #353

sendilkumarn · 2018-03-18T21:30:47Z

Followup and refined #275

This PR tries to make this webpack-cli repo into monorepo using lerna.

dhruvdutt

Please, clean yarn.lock and lerna-debug.log

dhruvdutt · 2018-03-18T21:41:14Z

lerna-debug.log

@@ -0,0 +1,34 @@
+0 silly input []


Add lerna-debug.log to .gitignore

dhruvdutt · 2018-03-20T14:33:09Z

packages/migrate/bannerPlugin/bannerPlugin.test.js

@@ -1,6 +1,6 @@
 "use strict";

-const defineTest = require("../../utils//defineTest");
+const defineTest = require("webpack-cli-utils//defineTest");


Why is there double slash //? It's even present on master.

It is in the master, I don't want to change this with this PR. And anyhow it is not higher priority.

@dhruvdutt Typo ^^

How is this even working? Shouldn't it throw an error? 😅
Might be doing sanitization.

dhruvdutt · 2018-03-20T14:35:35Z

packages/migrate/package.json

+{
+    "name": "webpack-cli-migrate",
+    "version":"2.0.12",
+    "description": "CLI for webpack & friends",


Can you please add description and readme (which would be visible on npm pages) for all packages?

That could be added later

dhruvdutt · 2018-03-20T15:44:41Z

packages/generate-loader/package.json

+    },
+    "dependencies": {      
+      "webpack-cli-generators": "2.0.12",
+      "yeoman-environment": "^2.0.0"


We're using yeoman-environment in all of the packages except webpack-cli-generators. Is there something that can be done about it to make it easier for maintaining a single version of it across all packages else we'll have to keep all versions of yeoman-environment in parity while upgrading in future.

What do you mean here?

@dhruvdutt I don't think it's possible. I totally understand your point and it would great to just change the version in one single place and that's it!

@sendilkumarn It means that if we have to upgrade the yeoman-environment we have to it everywhere (and we have to remember to do it). @dhruvdutt is suggesting if there's a way to update just in one place so we avoid problems (or less problems). I don't know much about how lerna works, maybe there's a solution for that

That is what lerna exactly tries to do, may be that is the whole reason to use something like lerna. add

I don't think we use yeoman in all packages

dhruvdutt · 2018-03-20T15:47:10Z

packages/generators/package.json

+      "log-symbols": "2.2.0",
+      "mkdirp": "^0.5.1",
+      "webpack-addons": "^1.1.5",
+      "webpack-cli-generators": "2.0.12",


webpack-cli-generators depends on webpack-cli-generators? ↩️
Recursive dependency. 😄

dhruvdutt · 2018-03-20T15:50:23Z

packages/generators/package.json

+      "webpack-addons": "^1.1.5",
+      "webpack-cli-generators": "2.0.12",
+      "webpack-cli-init": "2.0.12",
+      "webpack-cli-utils": "2.0.12",


Why are we using locked package versions everywhere? "2.0.12"?

Also, we're going to keep the versions of all packages in parity, right? Upgrade to one will cause all of the packages to bump versions.

That is the default config with lerna and lets do it in this way.

dhruvdutt · 2018-03-20T15:57:56Z

package.json

@@ -15,10 +15,14 @@
  "engines": {
    "node": ">=6.11.5"
  },
+  "private": true,


Why do all the packages have private flag?

would'nt this cause lerna to not publish this package? Or if i misunderstood the docs it won't publish just packages inside packages/?

Yeah that's true, we must update them

dhruvdutt · 2018-03-20T16:35:02Z

packages/migrate/package.json

+      "webpack-cli-utils": "2.0.12",
+      "yeoman-environment": "^2.0.0",
+      "yeoman-generator": "^2.0.3"
+    }


There seems to be a lot of extraneous dependencies everywhere like cross-spawn, global-modules, got, yeoman-environment which can be cleaned.

prettier is part of utils - runPrettier. Shouldn't be included here.
We don't use yeoman-generator as well in migrate.

dhruvdutt · 2018-03-20T16:38:24Z

packages/utils/package.json

+      "global-modules": "^1.0.0",
+      "got": "^8.2.0",
+      "jscodeshift": "^0.5.0",
+      "p-each-series": "^1.0.0",


Please clean extraneous dependencies here as well as in other packages.

dhruvdutt · 2018-03-20T16:43:56Z

package.json

@@ -111,6 +115,7 @@
  },
  "devDependencies": {


We need to start cleaning dependencies and devDependencies from the main package.
For instance inquirer is used only while migration and carries a weight of 815.9 kB.

We should better shift dependencies under the packages it is required and lighten up webpack-cli package.

as long as you move them into a separate repo, I think this doesn't make any sense. replacing inquirer for CLI is not advisable AFAICT.

+1 @sendilkumarn

dhruvdutt · 2018-03-20T16:47:20Z

@sendilkumarn I've resolved the conflicts and made a PR. Please check #1.

ematipico · 2018-03-20T22:09:05Z

@sendilkumarn Could you provide some context/description to your PR please? I know the title is self explaining but it would be nice to give a description of the work you've done - which is a lot! - so also the rest of the community can have a better understanding

sendilkumarn · 2018-03-20T22:30:58Z

@ematipico my bad, this was described in the old PR #275 and it was discussed across. But, yeah updated the ticket.

evenstensberg · 2018-03-21T07:53:25Z

#261

ematipico · 2018-03-21T10:51:39Z

Awesome job @sendilkumarn !

ematipico · 2018-04-04T12:59:37Z

We got some conflicts :(
Once resolved, we can merge!

sendilkumarn · 2018-04-04T14:05:06Z

@ematipico done

ematipico

Just flagged some changes to apply, mainly to remove the 'private' flag from the package.json files

ematipico · 2018-04-04T20:22:16Z

package.json

@@ -15,10 +15,14 @@
  "engines": {
    "node": ">=6.11.5"
  },
+  "private": true,


Yeah that's true, we must update them

ematipico · 2018-04-04T20:23:11Z

packages/generate-plugin/package.json

+    "version":"2.0.12",
+    "description": "CLI for webpack & friends",
+    "license": "MIT",
+    "private": true,


Also here, we have to get rid of the private: true

ematipico · 2018-04-04T20:23:26Z

packages/generators/package.json

+    "version":"2.0.12",
+    "description": "CLI for webpack & friends",
+    "license": "MIT",
+    "private": true,


ematipico · 2018-04-04T20:23:38Z

packages/init/package.json

+    "version":"2.0.12",
+    "description": "CLI for webpack & friends",
+    "license": "MIT",
+    "private": true,


ematipico · 2018-04-04T20:23:58Z

packages/migrate/package.json

+    "version":"2.0.12",
+    "description": "CLI for webpack & friends",
+    "license": "MIT",
+    "private": true,


ematipico · 2018-04-04T20:24:10Z

packages/utils/package.json

+    "version":"2.0.12",
+    "description": "CLI for webpack & friends",
+    "license": "MIT",
+    "private": true,


matheus1lva · 2018-04-04T20:27:50Z

packages/generate-loader/package.json

+    "version":"2.0.12",
+    "description": "CLI for webpack & friends",
+    "license": "MIT",
+    "private": true,


to be removed as the others.

webpack-bot · 2018-04-04T20:42:56Z

@sendilkumarn Thanks for your update.

I labeled the Pull Request so reviewers will review it again.

@ematipico Please review the new changes.

webpack-bot · 2018-04-04T20:58:01Z

@sendilkumarn Please review the following output log for errors:

> webpack-cli@2.0.13 travis:integration /home/travis/build/webpack/webpack-cli
> nyc jest && npm run reportCoverage

 PASS  packages/utils/resolve-packages.test.js (13.563s)
 PASS  packages/utils/npm-exists.test.js (19.849s)
 PASS  packages/utils/package-manager.test.js (21.088s)
 PASS  packages/utils/is-local-path.test.js (19.61s)
 PASS  packages/init/transformations/top-scope/top-scope.test.js (15.094s)
 PASS  packages/generators/loader-generator.test.js (28.087s)
 PASS  packages/init/transformations/output/output.test.js (29.708s)
 PASS  packages/migrate/removeDeprecatedPlugins/removeDeprecatedPlugins.test.js (33.436s)
  ● Console

    console.log packages/migrate/removeDeprecatedPlugins/removeDeprecatedPlugins.js:37
      
      Please remove deprecated plugins manually. 
      See https://webpack.js.org/guides/migrating/ for more information.
    console.log packages/migrate/removeDeprecatedPlugins/removeDeprecatedPlugins.js:37
      
      Please remove deprecated plugins manually. 
      See https://webpack.js.org/guides/migrating/ for more information.
    console.log packages/migrate/removeDeprecatedPlugins/removeDeprecatedPlugins.js:37
      
      Please remove deprecated plugins manually. 
      See https://webpack.js.org/guides/migrating/ for more information.

 PASS  packages/init/transformations/plugins/plugins.test.js (33.899s)
 PASS  packages/migrate/extractTextPlugin/extractTextPlugin.test.js (5.214s)
 PASS  packages/init/transformations/mode/mode.test.js (33.787s)
 PASS  packages/init/transformations/resolveLoader/resolveLoader.test.js (34.563s)
 PASS  packages/init/transformations/performance/performance.test.js (34.844s)
 PASS  packages/init/transformations/devServer/devServer.test.js (34.623s)
 PASS  packages/init/transformations/devtool/devtool.test.js (35.285s)
 PASS  packages/migrate/bannerPlugin/bannerPlugin.test.js (15.49s)
 PASS  packages/migrate/resolve/resolve.test.js (6.514s)
 PASS  packages/init/transformations/watch/watchOptions.test.js (35.447s)
 PASS  packages/init/transformations/watch/watch.test.js (36.092s)
 PASS  packages/init/transformations/context/context.test.js (34.184s)
 PASS  packages/init/transformations/stats/stats.test.js (36.171s)
 PASS  packages/migrate/removeJsonLoader/removeJsonLoader.test.js (35.959s)
 PASS  packages/init/transformations/node/node.test.js (34.913s)
 PASS  packages/migrate/outputPath/outputPath.test.js (15.823s)
 PASS  packages/init/transformations/target/target.test.js (35.289s)
 PASS  packages/migrate/loaderOptionsPlugin/loaderOptionsPlugin.test.js (35.198s)
 PASS  packages/utils/ast-utils.test.js (38.454s)
 PASS  packages/init/transformations/module/module.test.js (37.403s)
 PASS  packages/init/transformations/resolve/resolve.test.js (36.861s)
 PASS  packages/migrate/uglifyJsPlugin/uglifyJsPlugin.test.js (16.709s)
 PASS  packages/init/transformations/externals/externals.test.js (37.627s)
 PASS  packages/init/transformations/entry/entry.test.js (37.425s)
 PASS  packages/migrate/index.test.js (37.916s)
 PASS  packages/migrate/loaders/loaders.test.js (37.828s)
 PASS  packages/init/transformations/other/other.test.js (40.79s)
 PASS  packages/utils/npm-packages-exists.test.js (39.663s)
 FAIL  test/BinTestCases.test.js (196.593s)
  ● Console

    console.log test/BinTestCases.test.js:170
      ### stderr ###
      
    console.log test/BinTestCases.test.js:171
      ### stdout ###
      Hash: 2257fa43da59095809e2
      Version: webpack 4.5.0
      Time: 95ms
      Built at: 4/4/2018 8:56:34 PM
      
      WARNING in configuration
      The 'mode' option has not been set, webpack will fallback to 'production' for this value. Set 'mode' option to 'development' or 'production' to enable defaults for each environment.
      You can also set it to 'none' to disable any default behavior. Learn more: https://webpack.js.org/concepts/mode/
      
      ERROR in Entry module not found: Error: Can't resolve './src' in '/home/travis/build/webpack/webpack-cli/test/binCases/no-options/none'
      

  ● BinTestCases › /no-options/none › should run successfully

    expect(string).toContain(value)
    
    Expected string:
      ""
    To contain value:
      "ERROR in Entry module not found"

      11 | 	expect(stdout[6]).toContain("The \'mode\' option has not been set");
      12 | 	expect(stdout[7]).toContain("");
    > 13 | 	expect(stdout[8]).toContain("ERROR in Entry module not found");
      14 | 	expect(stderr).toHaveLength(0);
      15 | };
      16 | 
      
      at testAssertions (test/binCases/no-options/none/stdin.js:13:20)
      at Object.<anonymous> (test/BinTestCases.test.js:168:7)

�[999D�[K
=============================== Coverage summary ===============================
Statements   : 77.18% ( 1268/1643 )
Branches     : 62.15% ( 509/819 )
Functions    : 75.48% ( 314/416 )
Lines        : 77.15% ( 1256/1628 )
===

See complete report here.

evenstensberg · 2018-04-10T12:48:47Z

@sendilkumarn Can you merge with next and provide us with steps to test the monorepo?

sendilkumarn · 2018-04-15T19:55:13Z

@ev1stensberg I would like to locally link & use this branch for testing. | release a next version with this branch

Then go ahead and create / update any project with CLI ( init | migrate etc.,)
That should be the first step.

The next should be use only what is needed (i.e only those packages that are needed)

Then finally, we should make webpack-cli as a holder and make use of sub-packages wherever necessary. (In other words, add private: true in base package.json)

ematipico · 2018-04-16T06:19:03Z

All the packages have to be pushed on npm right? Same for the next branch

sendilkumarn · 2018-04-16T07:09:57Z

Yes. Unless we want to publish webpack-cli too.

sendilkumarn added 3 commits March 18, 2018 21:53

chore(monorepo): move to lerna

ec6cc38

chore(monorepo): use commands as normal instead of package

bdafce0

fix(revert): packagejson

3dd244b

webpack-bot added PR: unreviewed PR: CI-not-ok labels Mar 18, 2018

dhruvdutt reviewed Mar 18, 2018

View reviewed changes

lerna-debug.log Outdated

@@ -0,0 +1,34 @@

0 silly input []

Copy link

Member

dhruvdutt Mar 18, 2018 •

edited

Choose a reason for hiding this comment

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

Add lerna-debug.log to .gitignore

sendilkumarn added 4 commits March 18, 2018 22:53

chore(monorepo): prune files and update eslint rules

059d6f9

chore(monorepo): fix the no-missing-require error

8ef1804

chore(monorepo): fix typo

a1d5406

chore(monorepo): fix typo

a6a2715

webpack-bot added PR: CI-ok and removed PR: CI-not-ok labels Mar 18, 2018

dhruvdutt requested changes Mar 20, 2018

View reviewed changes

webpack-bot added PR: reviewed-changes-requested and removed PR: unreviewed labels Mar 20, 2018

Merge branch 'master' into lerna-workspace

e9bc6eb

webpack-bot added PR: review-outdated PR: CI-not-ok and removed PR: reviewed-changes-requested PR: CI-ok labels Mar 20, 2018

chore(monorepo): prune package json

64cfef7

sendilkumarn force-pushed the lerna-workspace branch from ccd9e15 to 64cfef7 Compare March 20, 2018 19:52

webpack-bot added PR: CI-ok and removed PR: CI-not-ok labels Mar 20, 2018

montogeek mentioned this pull request Mar 29, 2018

cli(refactor): improve folder structure #371

Merged

sendilkumarn changed the base branch from master to next April 2, 2018 07:38

ematipico added this to the Vatican (v3) milestone Apr 4, 2018

sendilkumarn added 2 commits April 4, 2018 16:00

Merge branch 'next' into lerna-workspace

2738a1e

chore(monorepo): merge package lock json

d4c7c5d

webpack-bot added PR: CI-not-ok and removed PR: CI-ok labels Apr 4, 2018

ematipico suggested changes Apr 4, 2018

View reviewed changes

webpack-bot added PR: reviewed-changes-requested and removed PR: review-outdated labels Apr 4, 2018

matheus1lva reviewed Apr 4, 2018

View reviewed changes

chore(monorepo): updated package.json

ff6c371

webpack-bot added PR: review-outdated and removed PR: reviewed-changes-requested labels Apr 4, 2018

chore(monorepo): fix plugin package.json

3c822cf

matheus1lva approved these changes Apr 4, 2018

View reviewed changes

webpack-bot added PR: reviewed-approved and removed PR: review-outdated labels Apr 4, 2018

sendilkumarn merged commit bdf7e5a into webpack:next Apr 15, 2018

Lerna workspace #353

Lerna workspace #353

Conversation

sendilkumarn commented Mar 18, 2018 • edited

dhruvdutt left a comment

Choose a reason for hiding this comment

dhruvdutt Mar 18, 2018 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sendilkumarn Mar 20, 2018 • 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

matheus1lva Apr 4, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhruvdutt Mar 20, 2018 • 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

dhruvdutt commented Mar 20, 2018 • edited

ematipico commented Mar 20, 2018

sendilkumarn commented Mar 20, 2018

evenstensberg commented Mar 21, 2018

ematipico commented Mar 21, 2018

ematipico commented Apr 4, 2018

sendilkumarn commented Apr 4, 2018

ematipico left a 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

webpack-bot commented Apr 4, 2018

webpack-bot commented Apr 4, 2018

evenstensberg commented Apr 10, 2018

sendilkumarn commented Apr 15, 2018 • edited

ematipico commented Apr 16, 2018 • edited

sendilkumarn commented Apr 16, 2018

sendilkumarn commented Mar 18, 2018 •

edited

dhruvdutt Mar 18, 2018 •

edited

sendilkumarn Mar 20, 2018 •

edited

matheus1lva Apr 4, 2018 •

edited

dhruvdutt Mar 20, 2018 •

edited

dhruvdutt commented Mar 20, 2018 •

edited

sendilkumarn commented Apr 15, 2018 •

edited

ematipico commented Apr 16, 2018 •

edited