Skip to content

Lock packages best practices #9381

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

Closed
magemello opened this issue Jan 25, 2018 · 20 comments
Closed

Lock packages best practices #9381

magemello opened this issue Jan 25, 2018 · 20 comments

Comments

@magemello
Copy link

magemello commented Jan 25, 2018

Hi,

My question is related to the bug reported in the issue #9276 and more an educational topic for myself and I hope for the community.

I saw from the @filipesilva comment that is advised to release also the lockfile, which is fine for me, this will make sure that the software that I release will work at the same way on my machine and on the next person that will install it (good).

My question is more on how you as Angular CLI team make sure that the Angular CLI will work at the same way on my machine and on your machine?
Let's suppose that you test your software and all is fine...now in between your release and me using it, one of your dependencies release an update that changes the behaviour of your software, not necessarily breaks it, what stops this thing from happening?

If what I'm describing is correct, I would also be interested in understanding why during the release phase you are not locking the dependencies in the package.json file?

Thank you

Versions

<!--
Output from: `ng --version`.
If nothing, output from: `node --version` and `npm --version`.
  Windows (7/8/10). Linux (incl. distribution). macOS (El Capitan? Sierra?)
-->
Your global Angular CLI version (1.6.5) is greater than your local
version (1.5.0). The local Angular CLI version is used.

To disable this warning use "ng set --global warnings.versionMismatch=false".

    _                      _                 ____ _     ___
   / \   _ __   __ _ _   _| | __ _ _ __     / ___| |   |_ _|
  / △ \ | '_ \ / _` | | | | |/ _` | '__|   | |   | |    | |
 / ___ \| | | | (_| | |_| | | (_| | |      | |___| |___ | |
/_/   \_\_| |_|\__, |\__,_|_|\__,_|_|       \____|_____|___|
               |___/
    
Angular CLI: 1.5.0
Node: 8.5.0
OS: darwin x64
Angular: 5.0.0
... animations, common, compiler, compiler-cli, core, forms
... http, language-service, platform-browser
... platform-browser-dynamic, router

@angular/.DS_Store: error
@angular/cdk: 5.0.0-rc0
@angular/cli: 1.5.0
@angular/flex-layout: 2.0.0-beta.10
@angular/material-moment-adapter: 5.1.0
@angular/material: 5.0.0-rc0
@angular-devkit/build-optimizer: 0.0.41
@angular-devkit/core: 0.0.28
@angular-devkit/schematics: 0.0.51
@ngtools/json-schema: 1.1.0
@ngtools/webpack: 1.8.0
@schematics/angular: 0.1.16
typescript: 2.4.2
webpack: 3.8.1

Repro steps

  • Install angular-cli 1.5
  • ng new test-project
  • ng serve

Observed behavior

<!-- Normally this includes a stack trace and some more information. -->
> module.js:540
>     throw err;
>     ^
> 
> Error: Cannot find module '@angular-devkit/core'
>     at Function.Module._resolveFilename (module.js:538:15)
>     at Function.Module._load (module.js:468:25)
>     at Module.require (module.js:587:17)
>     at require (internal/module.js:11:18)
>     at Object.<anonymous> (C:\Users\xxx\AppData\Roaming\npm\node_modules\@angular\cli\node_modules\@angular-devkit\schematics\src\tree\virtual.js:10:16)
>     at Module._compile (module.js:643:30)
>     at Object.Module._extensions..js (module.js:654:10)
>     at Module.load (module.js:556:32)
>     at tryModuleLoad (module.js:499:12)
>     at Function.Module._load (module.js:491:3)

Desired behavior

Project starts

Mention any other details that might be useful (optional)

@filipesilva
Copy link
Contributor

filipesilva commented Jan 25, 2018

There is nothing we can do on a release that would truly lock dependencies. Dependencies can only be locked on a consumer's side via lockfiles.

To be clear though, the error you are describing is reported in #9283 (comment) and we fixed it via new releases all the way back to 1.4.

Let's consider what would happen if we tried somehow pin dependencies. So during the release process of Angular CLI we go through all packages in the CLI's package.json and pin them. That means changing any loose dependency (like some-package@~1.2.3) to a fixed one (some-package@1.2.3).

Now Angular CLI has pinned it's dependencies. But any indirect dependencies (dependencies of our dependencies) are not pinned. We don't control those packages. some-package@1.2.3 could depend on another-package@^3.4.5, and you can get different versions of another-package even though the CLI has some-package@1.2.3 pinned.

So to answer your question:

Let's suppose that you test your software and all is fine...now in between your release and me using it, one of your dependencies release an update that changes the behaviour of your software, not necessarily brakes it, what stops this thing from happening?

As long as there is any loose dependency anywhere in the dependency tree, nothing stops this from happening.

That is the cost of using loose versioning: you automatically get bugfixes and new features, but you also can get bugs introduced by new versions.

You might think that this is madness but consider that any software version has bugs, otherwise there would be no point in releasing a new version with bugfixes.

One alternative is to convince every package author to pin their dependencies. Besides being impractical, it also means that whenever a package gets a bugfix everyone has to do a new release.

If you consider that most npm library has a good few hundreds of total (direct + indirect) dependencies, and that when any lib changes then all the others that use it need to update as well, it doesn't take more than a couple of indirect dependencies until literally thousands of packages are need an update as a result of a bugfix in a single package.

Another alternative is to have package consumers use lockfiles and treat dependency updates as a serious change. A lockfile gives you the guarantee that multiple installs with the same lockfile will give you the same dependency tree. In other words, as long as you don't update any package, installs will get you always the same thing.

Once you update the lockfile that can change, but package managers usually try to do the minimum changes to the lockfile possible on updates. This limits how many packages change and thus reduces risk.

But realistically speaking you are always using a couple of orders of magnitude more code in dependencies code than your own source code. Updating a dependency can mean several megabytes of changed code inside node_modules, likely far more changes than own source code has ever had.

It is not reasonable to review all of these changes because of the time and knowledge it would take. Nor does reviewing them scale, because each update would cause all consumers to have to review changed code.

So when dependending on community packages one generally has to trust them to produce mostly bug free releases and address bugs as they are discovered. This in addition of testing your own code to make sure it always works as you expect.

I would like better ways to control indirect dependencies though, like I requested in npm/npm#19512.

It sucks to have no changes in your source code and have builds fail, but the solution for that is to use lockfiles. This solution exists right now and if you don't use it you are at risk.

It also sucks to make a small change in your package.json and have builds fail. But you should realize that package.json and the lockfile are part of your source files, and that changing those files results in a massive amount of changed code in node_modules. Just because it's not on git doesn't mean you're not using it. Then you realize that you are making the choice to trust package maintainers instead of spending a good thousands of hours reviewing the changed dependencies.

TLDR: use lockfiles, test your code, update carefully, report bugs, hope for the best.

@magemello
Copy link
Author

Thank you Filipe for taking the time to write a so detailed answer.

I agree with you that we should push to have a better system, the lock file is a good workaround but not a consistent system. The problem again here is the "time dependency" I can lock my app to the dependencies I'm installing when I'm consuming my dependencies but I can't be sure I'm installing the same indirect dependencies that my dependencies have installed when they tested their release.

My opinion is that loose dependencies are against semantic versioning, and should be used only for development not for released software. The fact of getting any changed behaviour in your software "for free" (new bug, bug fixes, new features) without update any of the numbers in your versions it's really bad and against semantic.

I personally think npm should force to pin all the versions in case of a release and allow loose dependencies only for @beta & @Alpha. As it is now we can not really say that npm supports semantic version.

Anyway going back to Angular CLI, should at least the generated app include the lock-file.json you used when you tested it?

I also think that big projects like Angular have the duty to lead on best practices, and maybe start pinning all your dependencies when you release will push more projects doing it.

@filipesilva
Copy link
Contributor

I don't agree with what you're saying about semantic versioning. Semantic versioning, as defined by https://semver.org/, is actually all about making sure loose dependencies work well by limiting breaking changes to major versions.

In systems with many dependencies, releasing new package versions can quickly become a nightmare. If the dependency specifications are too tight, you are in danger of version lock (the inability to upgrade a package without having to release new versions of every dependent package). If dependencies are specified too loosely, you will inevitably be bitten by version promiscuity (assuming compatibility with more future versions than is reasonable). Dependency hell is where you are when version lock and/or version promiscuity prevent you from easily and safely moving your project forward.
...
As a responsible developer you will, of course, want to verify that any package upgrades function as advertised. The real world is a messy place; there’s nothing we can do about that but be vigilant. What you can do is let Semantic Versioning provide you with a sane way to release and upgrade packages without having to roll new versions of dependent packages, saving you time and hassle.

Not all packages on npm follow semantic versioning though.

I also don't think it makes any sense to pin all dependencies on releases. Any bugfix would require a huge amount of releases to propagate. This doesn't sound very bad small bugfixes but big ones, especially security ones, would leave everyone at risk.

We had some internal discussion about generating projects with lockfile for when we tested that release. I'm not sure what the problems with that approach were but @hansl might know more.

@OCCGU
Copy link

OCCGU commented Jan 25, 2018

Isn't the problem in the OP a result of the package angular-devkit/schematics claiming it didn't have the package angular-devkit/core as a dependency while it actually has one or more imports from it? This was changed in one of the recent releases, if I read correctly, because you're now supposed to install a root project which includes /core instead of /schematics directly
Maybe they should've done a major release and not a minor if significant changes that render it unusable in some configurations are released.

@magemello
Copy link
Author

magemello commented Jan 25, 2018

https://semver.org/#spec-item-3

Once a versioned package has been released, the contents of that version MUST NOT be modified. Any modifications MUST be released as a new version.

For me indirect dependencies are an internal part of the package I'm installing and if they change the behaviour of a released package they are against the spec item number 3.

And again here for me, the problem is the "time dependency".
at:

  • T1 npm install -> npm start -> all good
  • T2 npm install -> npm start -> error

@DenysVuika
Copy link

@filipesilva Just to give some context. We release a project that has been tested and confirmed to work properly. A couple of days later, at the conference, it suddenly stops working for developers and consumers, for everyone.

So we have spent a lot of time trying to find out why everything broke after simple npm install and why all CLI commands not working without adding "devkit/core" and "@angular-devkit/schematics": "0.0.34" to the package. And face the need re-publishing everything we released just because someone in your team broke library other CLI library depends on in a non-strict/pinned way.

The question is, how we can now be sure you are not going to break everything once per day with "sorry, guys, you need to upgrade"? One of the suggestions, as @magemello mentioned - that you pin all the dependencies the CLI tools have, and those our projects get indirectly.

@clydin
Copy link
Member

clydin commented Jan 25, 2018

@magemello In you're example:

T1 npm install -> npm start -> all good
T2 npm install -> npm start -> error

With the use of a package lock file in the project this won't happen, as it records the specific versions of ALL dependencies originally installed. This includes direct and transitive dependencies.
Note that manually updating any dependencies (either through changes to package.json or npm update or similar) can (and mostly likely will) cause the lock file to be updated.

@clydin
Copy link
Member

clydin commented Jan 25, 2018

Also, some further information on lock files can be found at the following locations:

@filipesilva
Copy link
Contributor

@OCCGU yes, that problem was the result of some bad version management on our part in regards to our @angular-devkit/* packages. Those packages are not yet 1.0 and there was change that inadvertenly broke consumers, like the CLI itself. The releases I listed in #9283 have those dependencies pinned and we're going to adopt a stricter version management on those packages going forward.

@magemello you can interpret it like that, but I think then you're trying very hard to not consider loose dependencies. That spec does allow for loose dependencies and talks about how they are useful (emphasis mine):

In systems with many dependencies, releasing new package versions can quickly become a nightmare. If the dependency specifications are too tight, you are in danger of version lock (the inability to upgrade a package without having to release new versions of every dependent package). If dependencies are specified too loosely, you will inevitably be bitten by version promiscuity (assuming compatibility with more future versions than is reasonable). Dependency hell is where you are when version lock and/or version promiscuity prevent you from easily and safely moving your project forward.
...
A simple example will demonstrate how Semantic Versioning can make dependency hell a thing of the past. Consider a library called “Firetruck.” It requires a Semantically Versioned package named “Ladder.” At the time that Firetruck is created, Ladder is at version 3.1.0. Since Firetruck uses some functionality that was first introduced in 3.1.0, you can safely specify the Ladder dependency as greater than or equal to 3.1.0 but less than 4.0.0. Now, when Ladder version 3.1.1 and 3.2.0 become available, you can release them to your package management system and know that they will be compatible with existing dependent software.

I don't think the example you give in your last comment is a good one for the same reasons that @clydin mentioned. I think the specific problem that is not addressed by package lock is with ng new, when creating a new package:

  • T1 ng new -> npm start -> all good
  • T2 ng new -> npm start -> error

@DenisVuyka I don't quite follow your example. If you're using a lockfile, running npm install will always use the same dependencies. Regarding the suggestion of pinning all dependencies the CLI uses, I have to refer you back to my first comment in this thread: #9381 (comment).

@magemello
Copy link
Author

On the semantic we can disagree that we diasgree :)

If you don't include the generated package-lock.json in the generated app...my example is still likely to happen.

@clydin
Copy link
Member

clydin commented Jan 25, 2018

The lock file is generated during the initial npm install/yarn/etc.

@magemello
Copy link
Author

@clydin you are focusing on the post app generation....but my problem is with post Angular CLI release.
Angular CLI release -> Angular CLI dependency changes and brakes the CLI -> you generate an app -> your app doesn't work.

@clydin
Copy link
Member

clydin commented Jan 25, 2018

That's because having an existing application (in production for instance) break during a CI build is a critical failure. Proper package locking prevents this behavior and was the primary concern for many experiencing these issues.
Having a newly generated, unreleased, project temporarily break (while definitely a problem) is not near the same criticality. It is also relevant to note that the potential breakage is actually a result of the combination of having an older patch release globally installed and the current behavior of pinning the exact version of the CLI within newly generated projects; resulting in the latest patch release with potentially critical defect fixes not being used in the new project.

@DenysVuika
Copy link

@clydin blaming everyone for not having/maintaining lock files won't solve the problem community is experiencing right now at this very moment with broken versions of your libs. The #9194 being closed fast without even ensuring it's actually fixed makes lots of people even sadder.

We can endlessly argue on theoretical aspects of versioning and lock files, but what should we tell our users and customers in terms of the current problem? Not everyone is using NPM 5 and lock files, not everyone had lock files before you guys screw things up.

@magemello
Copy link
Author

Hi @clydin for us is a criticality because we release the CLI as part of our software...we don't release only the app generated with it. And we can't have release time dependent...anyway as consequence we for sure are forced to release the package lock.

Anyway if we are saying that the package lock is the instrument to prevent this problem, I think generate the app with the same package lock used by the Angular CLI team to test it would avoid this problem...

@magemello
Copy link
Author

magemello commented Jan 26, 2018

@filipesilva & @hansl any chance you are going to include the package-lock in the generated app to make sure we are running the same software you tested?

@toddwseattle
Copy link

Is yarn any better at this stuff? I fell on this thread after npm installing a component and then having the cli consequently break it's dependencies (and it was on 'the latast' cli since its a small project started a few days ago). As near as I can tell, it generated a new package.json lock file and it went south from there. The only solution was to do what i like to call the 'npm cleanse' which is delete ./node_modules and the lock file and install. Package brittleness definitely increases the perceived learning curve things like angular that have alot of libs and scaffolding. No simple solution at this point; and having the cli definitely helps mask until it all blows up on you ;).

@OP-Klaus
Copy link

Shrinkwrapping would be nicer than package locking since it is "publishable"; it propagates through npm
https://docs.npmjs.com/cli/shrinkwrap

@alan-agius4
Copy link
Collaborator

Hello, we reviewed this issue and determined that it doesn't fall into the bug report or feature request category. This issue tracker is not suitable for support requests, please repost your issue on StackOverflow using tag angular-cli.

If you are wondering why we don't resolve support issues via the issue tracker, please check out this explanation.

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants