Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The dev-plugin-install & -create tasks should manage to dedupe CKE5 dependencies #43

Closed
Reinmar opened this issue Nov 26, 2015 · 13 comments
Milestone

Comments

@Reinmar
Copy link
Member

Reinmar commented Nov 26, 2015

I've just realised that the current way how we install plugins, by bypassing NPM a bit means that we'll have duplicated dependency and in a result a huge mess. In order to be able to keep all repos just once in the file system, we need to use npm link for everything (edit: turns out that it's not true).

Example:

  • you install ckeditor5-classiccreator,
  • this means cloning this repo to ../ and linking it in ckeditor5/node_modules/,
  • now, we need to run npm install in ckeditor5-classiccreator repo,
  • this means that we'll have as many grunt and some common dev modules installation as the number of plugins,
  • but what's worse – ckeditor5-classiccreator may require ckeditor5-toolbar which should also be installed once, even if it's used by 100 other plugins.

Therefore, every single CKEditor5 dependency needs to be kept in ../ automatically. Either from the beginning (from the moment when classiccreator needed it) or from the moment when someone wanted to start working on it.

@Reinmar
Copy link
Member Author

Reinmar commented Nov 27, 2015

I've been trying to imagine once again all the typical scenarios. I think the list below describe them well.

A. Cloning the ckeditor5 repo for the first time

  1. The developer clones the repo.
  2. The developer runs npm install inside it. NPM will now install all modules inside the top-level node_modules/ directory instead there's a collision between two packages (they require some package in two different versions).
  3. At this point the developer will have all CKEditor 5 plugins in ckeditor5/node_modules/, so we're fine.

B. Using development version of a chosen plugin (given NPM package name or Git URL)

  1. The developer performs all steps from scenario A.
  2. The developer runs gulp dev-install <plugin name or Git URL>.
  3. The task will:
    1. Get Git URL from NPM if plugin name given.
    2. Clone this repository to ../ (if it's not cloned already).
    3. If ckeditor5/node_modules/<plugin name> is taken, remove it.
    4. Execute ln -s ../../<plugin-dir> in ckeditor5/node_modules/.
    5. Add the dependency to ckeditor5/package.json.
    6. Run npm install in ckeditor5/ to install (in ckeditor5/node_modules/) all missing dependencies (including plugin's dependencies).

C. Using development version of a chosen plugin (given path to a plugin on your local machine)

  1. The developer performs all steps from scenario A.
  2. The developer runs gulp dev-install <plugin path>.
  3. The task will:
    1. If ckeditor5/node_modules/<plugin name> is taken, remove it.
    2. Execute ln -s <plugin path> in ckeditor5/node_modules/.
    3. Add the dependency to ckeditor5/package.json (you can set it to the given path).
    4. Run npm install in ckeditor5/ to install (in ckeditor5/node_modules/) all missing dependencies (including plugin's dependencies).

D. Adding a dependency to an already installed plugin

  1. The initial state is a result of scenario B or C. The plugin was installed using gulp dev-install.
  2. The developer adds some dependency to his/her plugin (either some 3rd party one or CKEditor 5 plugin).
  3. The developer has to run npm install in ckeditor5.
  4. There are two scenarios related to plugin's dependencies which are CKEditor 5 plugins that can happen now:
    • Either the plugin's dependency was already installed with gulp dev-install as well, so AFAICT NPM won't touch it (as it's already installed vel. linked).
    • The plugin's dependency wasn't installed with gulp dev-install yet so it will be installed as normal package. In this case, the developer can turn it into a dev version by calling gulp dev-install <dep-name>.

E. Cloning the ckeditor5 repo in the development mode

The expected result of this operation is that all CKEditor plugins specified in ckeditor5/package.json are installed in the development version (using a method from scenario B).

  1. The initial state is a result of scenario A.
  2. The developer runs gulp dev-init.
  3. The task gets all dependencies which are CKEditor 5 plugins (ckeditor5-*) from ckeditor5/package.json and installs them with gulp dev-install.

F. Updating a development version (linked plugins)

The expected result of this operation is that all CKEditor plugins specified in ckeditor5/package.json are first installed in the development mode (repeats scenario E) and then changes in all repositories are fetched, repositories are checked out to branches specified in ckeditor5/package.json (or master if no branch given).

  1. The initial state is a result of scenario E.
  2. The developer works in the ckeditor5 repo and e.g.:
    • specifies in ckeditor5/package.json that some plugins should be checked out to specific branches,
    • in the meantime other developers made commits to some plugins.
  3. The developer runs gulp dev-update.
  4. The task runs npm update in order to update 3rd party dependencies. (TODO: will it update ckeditor5's dependencies dependencies? E.g. dependencies of ckeditor5-foo plugin?).
  5. The task iterates over all CKEditor5 dependencies specified in package.json:
    1. If the plugin is not installed in the development mode yet, it should be.
    2. Changes should be fetched in the plugin repo.
    3. The branch in the plugin repo should be changed.

Notes

  • When linking we assume that the bottom-most directory name equals to the plugin name. We can make it more flexible in the future.
  • I think that scenarios E and F can be merged into a single one, although they represent two different moments in the developer's workflow.

@szymonkups
Copy link
Contributor

Thanks for the analysis, this looks like a way to go. However, I've manually performed steps from point D and it looks like npm is skipping modules that are linked with warning update-linked node_modules/ckeditor5-core needs updating to 0.0.1-0.dev from 0.0.1-0.dev but we can't, as it's a symlink. This means that no new depdependencies for linked plugin will be installed. Can you confirm?

@Reinmar
Copy link
Member Author

Reinmar commented Nov 27, 2015

This means that no new depdependencies for linked plugin will be installed. Can you confirm?

I think they were. But you must have this thing specified in your package.json. I'd need to check again, but yesterday I think it worked for me. I'm only not sure if in all cases (including "pck-name": "../local-path") or without the local path case.

@Reinmar
Copy link
Member Author

Reinmar commented Nov 27, 2015

Turns out that if in package.json we have:

dependencies: {
    "pck-name": "../local-path"
}

Then npm install works fine in a scenario that you've just added something to ../local-path/package.json because it naturally gets the most up to date version of that file. Unfortunately, when pointing to an NPM package, GitHub or Git URL then it will work only if that updated package.json was already pushed there (and published in case of NPM dep). Unfortunately this would make this workflow very inconvenient and error prone. When developing something (a PoC for instance) you don't want to be forced to push those changes. You are used to installing them e.g. by:

cd ckeditor5-classiccreator
npm install --save ckeditor5-toolbar

At this point in ckeditor5 you would call npm dedupe and this seems to be an option for us at it moves all the dependencies to ckeditor5.

There's a problem though – it works only for the first time. E.g. if ckeditor5-toolbar@1.0.0 was deduped and then it got an updated and ckeditor5-classiccreator started requiring version 2.0.0, running npm dedupe after upgrading that package in ckeditor5-classicreator directory won't override the previously dedupe version (1.0.0) with the new one as NPM thinks there's some conflict. Buuuuu... ;/

On the other hand, it's not that big issue. In your typical development workflow you either have all CKEditor 5 plugins installed through NPM and then you can simply do in ckeditor5 npm update, or, if you want to be sure rm -rf node_modules && npm install (which we used to do when there are some weird issues).

In the development mode you have most of the frequently changing modules cloned and symlinked, so pulling latest changes is enough to get the latest versions. Other ones (like 3rd party modules) you can update less often by "hard-resetting" the dev env (we can have a command for that).

So it seems that my workflow proposal needs some updates. I'll post them later.

@Reinmar
Copy link
Member Author

Reinmar commented Nov 27, 2015

@szymonkups had an idea that while running the tasks we can temporarily update package.json so CKEditor 5 plugins point to local directories and thanks to that all npm tools will work. This may be the simplest solution.

@szymonkups
Copy link
Contributor

PR includes:
A. It is working out-of-the-box.
B. Can be executed by grunt dev-install --plugin <NPM name or GitHub URL>. Branch information can be also added when GitHub URL is used: ckeditor/ckeditor5-core#t/84.
C. Can be executed by grunt dev-install --plugin <path on disk>.
D. Adding dependencies (when GitHub URL or NPM package name was used) must be done as described in previous comments with npm dedupe.
E. Can be executed by grunt dev-init.
F. In my opinion we need to rethink this scenario - we shouldn't force developers to put all ckeditor5- dependencies to developer mode when updating. For now I left the command grunt dev-update to scan for plugins already in dev mode, and calling git pull in each repository. Calling npm update in ckeditor5 is not updating linked dependencies dependencies, so npm dedupe must be used as in point D.

Tests can be started by npm run tests. Code coverage tool is started by npm run coverage.
Quite interesting conversation on NPM and updating local dependencies.

@szymonkups
Copy link
Contributor

I just realized that during plugin installation we need to install devDepencies inside each plugin. During plugin development some tasks need to be present inside plugin's directory - applying Git hooks, JSHint and JSCS checking. When we move all dependencies to CKEditor repository, we won't be able to run these tasks. Each plugin will also contain its own tests, so dependencies needed for testing should be present there too.
If we install devDependencies inside plugin directory I think that calling npm dedupe will move them to CKEditor repository.

@Reinmar
Copy link
Member Author

Reinmar commented Dec 11, 2015

Quite interesting conversation on NPM and updating local dependencies.

Could you perhaps sum up this conversation? I started reading it, but it's long :)

@szymonkups
Copy link
Contributor

Basically this thread shows what many different problems other developers have when they're dealing with NPM and local dependencies. Unfortunately, all the issues described there are somewhere near our problems but not exactly the same. Last comment there is written not that long ago and I was hoping that discussion will move further.

@Reinmar
Copy link
Member Author

Reinmar commented Dec 14, 2015

I've been trying to imagine once again all the typical scenarios. I think the list below describe them well.

Initial notes

  • In the development version of the project only packages which were installed using gulp dev-install or all package listed in ckeditor5/package.json if gulp dev-init was used will be used in their development versions. All other packages will be installed by npm.
  • The build task (or watcher) will decide which version of a package to use when more than one instance of this package was installed. This is due to the fact that npm dedupe totally fails on symlinks.
  • The build task may validate the tree of installed packages in the future, checking if there are no two versions of a single package. Such situation means that either:
    • the developer needs to run npm update on all dependencies,
    • there's a conflict between some packages as they are requiring two different versions of a single package.

A. Cloning the ckeditor5 repo for the first time

  1. The developer clones the repo.
  2. The developer runs npm install inside it. NPM will now install all modules inside the top-level node_modules/ directory instead there's a collision between two packages (they require some package in two different versions).
  3. At this point the developer will have all CKEditor 5 plugins in ckeditor5/node_modules/ thanks to automatic deduplication, so we're super fine.

B. Using development version of a chosen plugin (given NPM package name or Git URL)

  1. The developer performs all steps from scenario A.
  2. The developer runs gulp dev-install <plugin name or Git URL>.
  3. The task will:
    1. Get Git URL from NPM if plugin name given.
    2. Clone this repository to ../ (if it's not cloned already).
    3. Run npm install inside ../<plugin-dir> to install its dependencies (we do not care about deduping at this point). This is an important step because it ensures that all dependencies (incl. dev deps) of this plugin are available inside it.
    4. If ckeditor5/node_modules/<plugin name> is taken, remove it. Alternatively we could try to run npm uninstall <plugin-name> in order to remove from ckeditor5/node_modules/ all dependencies of this plugin which aren't required by other plugins.
    5. Execute ln -s ../../<plugin-dir> in ckeditor5/node_modules/.
    6. Add the dependency to ckeditor5/package.json.
  4. The builder will use:
    • TODO

C. Using development version of a chosen plugin (given path to a plugin on your local machine)

  1. The developer performs all steps from scenario A.
  2. The developer runs gulp dev-install <plugin path>.
  3. The task will:
    1. If ckeditor5/node_modules/<plugin name> is taken, remove it.
    2. Execute ln -s <plugin path> in ckeditor5/node_modules/.
    3. Add the dependency to ckeditor5/package.json (you can set it to the given path).
    4. Run npm install inside ../<plugin-dir> to install its dependencies.

D. Adding a dependency to an already installed plugin

  1. The initial state is a result of scenario B or C. The plugin was installed using gulp dev-install.
  2. The developer adds some dependency to his/her plugin (either some 3rd party one or CKEditor 5 plugin).
  3. The developer has to run npm install in the plugin directory.
  4. The builder will then find this dependency inside such plugin or inside some other plugin if it was already in use.

E. Cloning the ckeditor5 repo in the development mode

The expected result of this operation is that all CKEditor plugins specified in ckeditor5/package.json are installed in the development version (using a method from scenario B).

  1. The initial state is a result of scenario A.
  2. The developer runs gulp dev-init.
  3. The task gets all dependencies which are CKEditor 5 plugins (ckeditor5-*) from ckeditor5/package.json and installs them with gulp dev-install.

F. Updating a development version (linked plugins)

The expected result of this operation is that all CKEditor plugins specified in ckeditor5/package.json are first installed in the development mode (repeats scenario E) and then changes in all repositories are fetched, repositories are checked out to branches specified in ckeditor5/package.json (or master if no branch given).

  1. The initial state is a result of scenario E.
  2. The developer works in the ckeditor5 repo and e.g.:
    • specifies in ckeditor5/package.json that some plugins should be checked out to specific branches,
    • in the meantime other developers made commits to some plugins.
  3. The developer runs gulp dev-update.
  4. The task installs (gulp dev-install) all plugins which are specified in ckeditor5/package.json but weren't installed yet.
  5. The task fetches changes in all plugins listed in ckeditor5/package.json and checks out these repositories to correct branches.
  6. Optional (running npm update in ckeditor5 and all dev plugins will take awfully lot of time):
    1. The task runs npm update in ckeditor5 in order to update all dependencies which weren't installed in the development version.
    2. The task runs npm update in all plugins listed in package.json (so the plugins which are currently installed in their development versions) to update their dependencies.

Notes

  • When linking we assume that the bottom-most directory name equals to the plugin name. We can make it more flexible in the future.
  • I think that scenarios E and F can be merged into a single one, although they represent two different moments in the developer's workflow.

@szymonkups
Copy link
Contributor

I updated my pull request to latest scenarios described above. The dev-upate task can be started with option --npm-update in order to run npm update inside main and plugins repositories.

@Reinmar
Copy link
Member Author

Reinmar commented Dec 15, 2015

I've checked some of the tasks and our plan seems to work :). It's sad though how much time it will take to install the whole project in the dev version... Fortunately you'll need to do this just once and then add couple of modules from time to time.

@Reinmar
Copy link
Member Author

Reinmar commented Dec 15, 2015

Fixed by #46. Well done!

@Reinmar Reinmar closed this as completed Dec 15, 2015
@Reinmar Reinmar modified the milestone: 0.1.0 Mar 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants