Ckeditor5: Upgrade to lerna 3 or change it to something else

[Reinmar]

It introduced many backwards incompatible changes. We need to align our setup to it because people will be installing it instead of the support learna@2 due to how our docs install it.

Perhaps, the right moment to update all this will be when we'll release a new version of mgit2 because that guide will need to update then anyway.

[Reinmar]

There's one more thing that we'll need to change – how the deps are installed in ckeditor5 itself. See https://github.com/lerna/lerna/issues/878#issuecomment-414740900 and the four following comments.

[Reinmar]

We can also consider switching to Yarn workspaces: https://yarnpkg.com/blog/2017/08/02/introducing-workspaces/

[Reinmar]

@pomek, would you like to research how and if we could use Yarn? 👆

[pomek]

I will :)

[Reinmar]

Another thing to check would be https://github.com/pnpm/pnpm.

• https://dev.to/zkochan/pnpm-vs-lerna-filtering-in-a-multi-package-repository-587i
• https://github.com/pnpm/pnpm/issues/349
• https://pnpm.js.org/docs/en/workspace.html

Also regarding lerna and yarn:

• https://medium.com/@NareshBhatia/sharing-ui-components-with-lerna-and-yarn-workspaces-be1ebca06efe

[czerwonkabartosz]

We use https://github.com/pnpm/pnpm in CKEditor Cloud Services and it works really well. Performance is great for us and we do not have any problems with them.

[Reinmar]

My biggest issue with pnpm is that it's not very popular yet. Compare:

• https://www.npmjs.com/package/yarn
• https://www.npmjs.com/package/pnpm

It may be risky for us to use a tool which isn't well known because we also force other developers to use them.

[czerwonkabartosz]

My biggest issue with pnpm is that it's not very popular yet

Yes, I agree with that. We use it in CS, but it's a closed project and we do not have a problem with it.

We have to keep our fingers crossed for https://github.com/pnpm/pnpm and for @zkochan to make it more popular because the software is really great .

[zkochan]

If you will use pnpm in ckeditor, it is guaranteed that your libs will work with Yarn and npm. It is not true vice versa. (see pnpm’s strictness helps to avoid silly bugs).

Regarding popularity. It is true that we are not that popular yet. But we are heavily used: https://pnpm.js.org/en/users.html
Glitch uses pnpm on the back-end from the beginning of this year! It allowed them to give unlimited space for node_module to their users.

Also, if you'll have good suggestions for improving multi-package repository support in pnpm, we'll probably implement them pretty soon. I would love to make pnpm the best package manager for multi-package repos.

P.S. We'd be honored to be used by such a great project as CKEditor!

[Reinmar]

If you will use pnpm in ckeditor, it is guaranteed that your libs will work with Yarn and npm. It is not true vice versa. (see pnpm’s strictness helps to avoid silly bugs).

It's the type of issues that we indeed have from time to time, so it's great to see that pnpm would help here.

Aso, I realised that we make people install https://www.npmjs.com/package/mgit2 when they are supposed to work with our development environment, so asking people to install pnpm wouldn't hurt anyone. I also thought that perhaps we might be compatible with both tools (yarn and pnpm) at the same time which would be even nicer.

I'm really curious about the results of our research :)

[Reinmar]

I've just stumbled upon https://github.com/guigrpa/oao. I don't know if it could be of any help for us (and most likely it won't) but it's worth checking out.

[pomek]

Yarn

Allows installing packages the same (or even better) way how Lerna does.

It links packages located in packages/* and hoist it to the main package.

# ckeditor5/
$ ls -la node_modules/@ckeditor/

total 0
drwxr-xr-x    51 pomek  staff   1632 Sep 21 11:27 .
drwxr-xr-x  1496 pomek  staff  47872 Sep 21 11:28 ..
drwxr-xr-x     7 pomek  staff    224 Sep 21 11:27 ckeditor-cloud-services-collaboration
drwxr-xr-x    11 pomek  staff    352 Sep 21 11:27 ckeditor-cloud-services-core
lrwxr-xr-x     1 pomek  staff     41 Sep 21 11:27 ckeditor5-adapter-ckfinder -> ../../packages/ckeditor5-adapter-ckfinder
lrwxr-xr-x     1 pomek  staff     34 Sep 21 11:27 ckeditor5-alignment -> ../../packages/ckeditor5-alignment
lrwxr-xr-x     1 pomek  staff     32 Sep 21 11:27 ckeditor5-angular -> ../../packages/ckeditor5-angular
...
...
...
lrwxr-xr-x     1 pomek  staff     30 Sep 21 11:27 ckeditor5-utils -> ../../packages/ckeditor5-utils
lrwxr-xr-x     1 pomek  staff     31 Sep 21 11:27 ckeditor5-widget -> ../../packages/ckeditor5-widget

Why I said that it does the job better than Lerna? Because node_modules/ of packages does not contain anything. It just keeps git-hooks:

# ckeditor5/packages/ckeditor5-paragraph
$ ls -laR node_modules

total 0
drwxr-xr-x   3 pomek  staff   96 Sep 21 11:27 .
drwxr-xr-x  19 pomek  staff  608 Sep 21 11:27 ..
drwxr-xr-x   4 pomek  staff  128 Sep 21 11:27 .bin

node_modules/.bin:
total 0
drwxr-xr-x  4 pomek  staff  128 Sep 21 11:27 .
drwxr-xr-x  3 pomek  staff   96 Sep 21 11:27 ..
lrwxr-xr-x  1 pomek  staff   45 Sep 21 11:27 eslint -> ../../../../node_modules/eslint/bin/eslint.js
lrwxr-xr-x  1 pomek  staff   45 Sep 21 11:27 lint-staged -> ../../../../node_modules/lint-staged/index.js

AFAIK Lerna (^@2.0.0) keeps symlinks in packages' node_modules/ which makes a noise.

In order to work Yarn requires a small change in package.json:

{
  "workspaces": [
    "packages/*"
  ]
}

After calling yarn install all packages are linked to each other. All tests pass.

But Yarn isn't perfect. There are no things like publish or clean which Lerna has. It means that we must install both - Yarn and Lerna in order to have a working environment for ckeditor5-dev package. OTOH, writing a script that calls rm -rf node_modules/ for all packages (rm -rf packages/*/node_modules could work on Unix but I'm not sure) and a script that publishes packages on NPM shouldn't be too hard. We already have a lot of such kind of scripts so create another one won't be a problem.

yarn install with linking packages does the job in 155 sec. I don't know how much Lerna takes.

One thing to the consideration - what do we want to do with the yarn.lock file?

PNPM

Unfortunately, I don't have anything about this tool because I cannot run our project.

After creating a configuration file pnpm-workspace.yaml:

packages:
  - 'packages/*'

And calling pnpm recursive install it throws an error:

(node:30911) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 SIGINT listeners added. Use emitter.setMaxListeners() to increase limit

and finishes its job.

I will debug it but first expression yarn made better than pnpm.

[zkochan]

@pomek you should use this pnpm-workspace.yaml:

packages:
  - 'packages/**'

also, you may want to create a .npmrc in the root of the repo, with this config: link-workspace-packages = true (more details about this config here)

P.S. we are working on a huge update to pnpm that will make it even better for multi-package repos. However, it should already work fine, if you have any other issues, I'll have a look in the evening.

[pomek]

Thanks @zkochan for tips. Now my configs look like:

$ cat .npmrc
link-workspace-packages = true

$ cat pnpm-workspace.yaml
packages:
  - 'packages/**'

However:

• pnpm recursive install still ends with an error.
• pnpm install installs packages correctly.
• npm t -- -f media-embed still ends with ~an~ the error (pasted above). We had a lack of dependency but after merging https://github.com/ckeditor/ckeditor5-dev/pull/437 I was hoping that it helps. Unfortunately, the missing dependencies problem is a little deeper than I expected.

Below I pasted two logs:

# npm t -- -f media-embed

> [email protected] test /Users/pomek/Projects/ckeditor/ckeditor5
> node --max_old_space_size=4096 node_modules/@ckeditor/ckeditor5-dev-tests/bin/test.js "-f" "media-embed"

Error: Cannot find module 'browserslist'
npm ERR! Test failed.  See above for more details.

# pnpm run test -- -f media-embed

> [email protected] test /Users/pomek/Projects/ckeditor/ckeditor5
> node --max_old_space_size=4096 node_modules/@ckeditor/ckeditor5-dev-tests/bin/test.js "-f" "media-embed"

Error: Cannot find module 'browserslist'
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] test: `node --max_old_space_size=4096 node_modules/@ckeditor/ckeditor5-dev-tests/bin/test.js "-f" "media-embed"`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the [email protected] test script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/pomek/.npm/_logs/2018-09-24T08_51_35_538Z-debug.log

I decided to call npm install. But it was wrong way. After installing packages, test command produces more errors:

> [email protected] test /Users/pomek/Projects/ckeditor/ckeditor5
> node --max_old_space_size=4096 node_modules/@ckeditor/ckeditor5-dev-tests/bin/test.js "-f" "media-embed"

24 09 2018 10:54:37.186:ERROR [preprocess]: Can not load "webpack", it is not registered!
  Perhaps you are missing some plugin?
24 09 2018 10:54:37.192:ERROR [karma]: Error: No provider for "framework:mocha"! (Resolving: framework:mocha)
    at error (/Users/pomek/Projects/ckeditor/ckeditor5/node_modules/.registry.npmjs.org/di/0.0.1/node_modules/di/lib/injector.js:22:12)
    at Object.get (/Users/pomek/Projects/ckeditor/ckeditor5/node_modules/.registry.npmjs.org/di/0.0.1/node_modules/di/lib/injector.js:9:13)
    at Injector.get (/Users/pomek/Projects/ckeditor/ckeditor5/node_modules/.registry.npmjs.org/di/0.0.1/node_modules/di/lib/injector.js:54:19)
    at config.frameworks.forEach (/Users/pomek/Projects/ckeditor/ckeditor5/node_modules/.registry.npmjs.org/karma/3.0.0/node_modules/karma/lib/server.js:144:61)
    at Array.forEach (<anonymous>)
    at Server._start (/Users/pomek/Projects/ckeditor/ckeditor5/node_modules/.registry.npmjs.org/karma/3.0.0/node_modules/karma/lib/server.js:144:23)
    at Injector.invoke (/Users/pomek/Projects/ckeditor/ckeditor5/node_modules/.registry.npmjs.org/di/0.0.1/node_modules/di/lib/injector.js:75:15)
    at Promise.all.then.then (/Users/pomek/Projects/ckeditor/ckeditor5/node_modules/.registry.npmjs.org/karma/3.0.0/node_modules/karma/lib/server.js:123:24)
    at tryCatcher (/Users/pomek/Projects/ckeditor/ckeditor5/node_modules/.registry.npmjs.org/bluebird/3.5.2/node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (/Users/pomek/Projects/ckeditor/ckeditor5/node_modules/.registry.npmjs.org/bluebird/3.5.2/node_modules/bluebird/js/release/promise.js:512:31)
    at Promise._settlePromise (/Users/pomek/Projects/ckeditor/ckeditor5/node_modules/.registry.npmjs.org/bluebird/3.5.2/node_modules/bluebird/js/release/promise.js:569:18)
    at Promise._settlePromise0 (/Users/pomek/Projects/ckeditor/ckeditor5/node_modules/.registry.npmjs.org/bluebird/3.5.2/node_modules/bluebird/js/release/promise.js:614:10)
    at Promise._settlePromises (/Users/pomek/Projects/ckeditor/ckeditor5/node_modules/.registry.npmjs.org/bluebird/3.5.2/node_modules/bluebird/js/release/promise.js:694:18)
    at _drainQueueStep (/Users/pomek/Projects/ckeditor/ckeditor5/node_modules/.registry.npmjs.org/bluebird/3.5.2/node_modules/bluebird/js/release/async.js:138:12)
    at _drainQueue (/Users/pomek/Projects/ckeditor/ckeditor5/node_modules/.registry.npmjs.org/bluebird/3.5.2/node_modules/bluebird/js/release/async.js:131:9)
    at Async._drainQueues (/Users/pomek/Projects/ckeditor/ckeditor5/node_modules/.registry.npmjs.org/bluebird/3.5.2/node_modules/bluebird/js/release/async.js:147:5)

I'm not able to run our tests.

[pomek]

~One more argue against PNPM:~

Sorry, I am sorry for that. This sentence could be too aggressive. It could sound that is a blame of pnpm because I cannot run my command.

There is another issue that I couldn't understand.

ls -a packages/ckeditor5-autoformat/node_modules/.registry.npmjs.org/

ls -a packages/ckeditor5-engine/node_modules/.registry.npmjs.org/

A lot of packages are duplicated and they aren't hoisted or symlinked.

There is a dump of node_modules/ directory from the main package:

ls -a node_modules/

There are some packages but this isn't enough.

[pomek]

There are some packages but this isn't enough.

Lerna installs ~1500 packages. Hoisted packages are included.

[Reinmar]

A lot of packages are duplicated and they aren't hoisted or symlinked.

Aren't these all symlinks? Or isn't the whole .registry.npmjs.org a symlink to some global registry? I don't think that pnpm keeps anything duplicated – AFAIR, it does something completely opposite.

[Reinmar]

There is a dump of node_modules/ directory from the main package:

That's looks wrong indeed. The main package should have the most dependencies (Lerna's 1500 sounds reasonable). How do pnpm recursive install and pnpm install end?

[pomek]

Or isn't the whole .registry.npmjs.org a symlink to some global registry?

If it was a symlink, I never mention about it. Unfortunately, these directories contain real files (not symlinked).

[Reinmar]

• pnpm recursive install still ends with an error.

Is it the same error as previously? Did you try the new version mentioned by @zkochan? If not, we should report it to pnpm's issue tracker with some minimal steps.

[pomek]

How do pnpm recursive install

It ends with the error but pnpm install installs everything. This command also calls recursive install internally and it ends with a success.

[pomek]

Or isn't the whole .registry.npmjs.org a symlink to some global registry?

[Reinmar]

Then, perhaps all other packages contain symlinks to this registry? :D

[pomek]

And how should it work? :D All files inside the directories are linked? :D

[Reinmar]

No, just directories. You can even see this on your screenshot:

I guess you'll find the same pattern in all packages.

[pomek]

What is included in the registry I pasted above.

[Reinmar]

Hm... ok. So pnpm really duplicates the registry in every package. @zkochan, is this on purpose? Or is it something you plan to optimise?

[pomek]

P.S. we are working on a huge update to pnpm that will make it even better for multi-package repos. However, it should already work fine, if you have any other issues, I'll have a look in the evening.

@zkochan, I reported an issue – https://github.com/pnpm/pnpm/issues/1390 because I cannot use the code published on GitHub instead of the latest version published on npm.

[zkochan]

yes, as of version 2.16.1, pnpm creates an isolated node_modules for every package in the multi-package repo (with packages linked from the repo, when available)

We do have a new feature, which will create a shared node_modules in the root (I will try to publish it tonight). You'll have to set the shared-workspace-shrinkwrap config to true.

Disclaimer: This new feature is currently slow but if your monorepo is not too big (has not more than 10-20 packages) then it won't be noticeable. It will be fast for huge repos as well but I didn't have time yet to optimize it.

Also, you can check Rush which can be used with pnpm and has a smart workaround for the multiple node_modules problem. Rush is battle-tested by some huge projects at Microsoft and they frequently use it with pnpm for almost a year now. Rush also handles the publishing process for you.

cc @pgonzal

[pomek]

The latest version of the package (I mean pnpm installed from GitHub) installs dependencies properly. They have a common registry that is located in the main package.

I also understood what is located in node_modules/ directory. There are deps and dev-deps of given package. The registry contains all their dependencies.

But - npm t still throws an error:

> [email protected] test /Users/pomek/Projects/ckeditor/ckeditor5
> node --max_old_space_size=4096 node_modules/@ckeditor/ckeditor5-dev-tests/bin/test.js

Error: Cannot find module 'browserslist'
npm ERR! Test failed.  See above for more details.

So I need to debug manually which dependency has lack of the package and try to resolve the issue…

It's weird that Yarn, Lerna and npm work with an invalid set of packages defined in `package.json…

[pomek]

Below I share my .npmrc configuration:

link-workspace-packages = true
shared-workspace-shrinkwrap = true

[zkochan]

yes, pnpm is very strict. But it won't have the problem that yarn workspaces have currently

As they mention it in workspace Limitations & Caveats:

Be careful when publishing packages in a workspace. If you are preparing your next release and you decided to use a new dependency but forgot to declare it in the package.json file, your tests might still pass locally if another package already downloaded that dependency into the workspace root. However, it will be broken for consumers that pull it from a registry, since the dependency list is now incomplete so they have no way to download the new dependency. Currently there is no way to throw a warning in this scenario.

That being said, if it is too hard to fix the issues or you'd like to fix them later, you may use the shamefully-flatten config

[octogonz]

The Rush docs refer to this issue as "phantom dependencies". There's an article about it here:

https://rushjs.io/pages/advanced/phantom_deps/

PNPM's stricter installation model is currently the most complete solution. But it's mainly an problem that affects larger monorepos. Smaller projects don't seem to worry about it too much.

[pomek]

As I understood, npm@7 would like to introduce workspaces to itself.

Workspaces, as seen in Yarn, for management of certain kinds of monorepos.

But there is no date when npm@7 will be released. As far as we know, it will be "in the late summer and fall of 2018".

https://blog.npmjs.org/post/173239798780/beyond-npm6-the-future-of-the-npm-cli

[zkochan]

@pomek you may retry this with [email protected] (available through npm i -g pnpm@rc)

Just set this in a .npmrc in the root of the repo (as it is done in the pnpm monorepo):

link-workspace-packages = true
shared-workspace-shrinkwrap = true

and create a pnpm-workspace.yaml in the root of the repo.

pnpm will create a single lockfile and a single node_modules with all the hard linked packages. The performance issues were fixed since you last checked it and the pnpm monorepo now uses this new feature.