You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by Chris Brody <ch...@gmail.com> on 2018/09/17 11:14:16 UTC
[DISCUSS] When to do npm shrinkwrap?
Originated in another thread ([1]), moving into a new thread.
My understanding is that we should only do npm shrinkwrap if needed to
deal with a dependency problem in an end-user package such as
cordova-cli.
I would like to ask people to please use references whenever possible.
[1] https://lists.apache.org/thread.html/7f92561d382f143aaf49e083bbe215dcf95a3f4d8b6e3cbb6089a5f3@%3Cdev.cordova.apache.org%3E
On Mon, Sep 17, 2018 at 5:38 AM Oliver Salzburg
<ol...@gmail.com> wrote:
>
> +1 to shrinkwrap before publish
>
> On 2018-09-15 00:27, raphinesse@gmail.com wrote:
> > Please note that I'm not suggesting to commit shrinkwrap to the master of
> > every repository. That is what had been discussed in the mailing list
> > thread from 2014 that you shared. That option was dismissed then, and
> > rightly so. It is also strongly advised against in the npm docs.
> >
> > I suggest using it only in release branches of CLI or other non-library
> > releases, so they are immutable and don't mutate over time. _This is
> > exactly the intended use_. And it comes at no cost other than having to run
> > `npm shrinkwrap` right before the release.
> >
> > Plus, it allows us to override transitive dependency versions for a release
> > if absolutely necessary.
> >
> > Am Sa., 15. Sep. 2018 um 00:16 Uhr schrieb Jesse <pu...@gmail.com>:
> >
> >> I personally see shrinkwrap as a step in the wrong direction, I feel like
> >> it has been explored in the past, and the general consensus is that it is
> >> something to avoid.
> >> If committing package-lock is contentious, I am okay with skipping it, but
> >> we need to be careful to lock down our own dependencies and not use
> >> wildcards.
> >>
> >> We have flexibility in our current GitHub repos, which allow us to remix
> >> and freely link other dependent packages so we can debug right through our
> >> deps. I agree we need more stability around the release/publish steps so
> >> we can move more quickly through the collection of packages that make up a
> >> 'tools' release.
> >>
> >> @purplecabbage
> >> risingj.com
> >>
> >>
> >> On Fri, Sep 14, 2018 at 2:56 PM <ra...@gmail.com> wrote:
> >>
> >>> No. This won't fix anything. Plus it goes directly against npm's
> >>> recommendation. Please double check the docs for the use cases of
> >>> package-lock.json vs npm-shrinkwrap.json.
> >>>
> >>> Am Fr., 14. Sep. 2018 um 23:48 Uhr schrieb Chris Brody <
> >>> chris.brody@gmail.com>:
> >>>
> >>>> A really nice alternative may be to turn the generated
> >>>> package-lock.json into npm-shrinkwrap.json (using npm shrinkwrap
> >>>> command) then commit npm-shrinkwrap.json. Then I think any other npm
> >>>> install updates would update npm-shrinkwrap.json instead of
> >>>> package-lock.json. Could be more predictable and easier to understand.
> >>>>
> >>>> This was already discussed in 2014 [1], thanks to Jesse for the link in
> >>>> [2].
> >>>>
> >>>> Thanks for the suggestion to use npm shrinkwrap as a solution for
> >>>> cordova-cli 8.1.0 minor release in [2].
> >>>>
> >>>> [1]
> >>>>
> >> https://lists.apache.org/thread.html/99184622129935eb473e843e583bf6648faff279a014e8508cc2c660@1411013202@%3Cdev.cordova.apache.org%3E
> >>>> [2]
> >>>>
> >> https://lists.apache.org/thread.html/f89a074add24f2ace7006b0211cf43a47cc5c1a0a65932fc22515828@%3Cdev.cordova.apache.org%3E
> >>>> On Fri, Sep 14, 2018 at 3:53 PM Chris Brody <ch...@gmail.com>
> >>> wrote:
> >>>>> To be honest I have pretty limited experience with package lock file
> >>>>> and it is now starting to show. From Oliver's very unfortunate
> >>>>> experience I would conclude that this is something we should do very
> >>>>> carefully and not just on a whim. Some things I can think of:
> >>>>>
> >>>>> * always use recent version of npm such as npm@6.4.1 to generate or
> >>>>> update package-lock.json
> >>>>> * do not use. npm cache when generating or updating
> >> package-lock.json,
> >>>>> or use the npm cache with extreme care (also limited experience for
> >>>>> me)
> >>>>> * be extremely careful with assumptions; I think we should both
> >>>>> double-check the documentation and do our own experimentation, like I
> >>>>> did in <https://github.com/apache/cordova-cli/pull/325> to validate
> >> as
> >>>>> best we can
> >>>>> * semver package seems to be a major library package used by npm; we
> >>>>> should both read the documentation and experiment, ideally with its
> >>>>> own test cases
> >>>>>
> >>>>> On Fri, Sep 14, 2018 at 9:47 AM Oliver Salzburg
> >>>>> <ol...@gmail.com> wrote:
> >>>>>> The problems that appear when you have linked dependencies is that
> >>> npm
> >>>>>> will pick them up as being bundled and mark them as such in the
> >>>>>> lockfile. *However* this behavior has changed in the past. At one
> >>> point
> >>>>>> this affected any direct dependency, at another point it "only"
> >>>> affected
> >>>>>> dependencies of dependencies.
> >>>>>>
> >>>>>> Either way, the result is:
> >>>>>>
> >>>>>> a) a corrupted lockfile, which has dependencies marked as bundled
> >>>>>> b) a lockfile that lists incorrect versions, resulting from linking
> >>>>>> temporary development snapshots together
> >>>>>>
> >>>>>> When you use a local npm cache and you neglected to correctly
> >>>>>> parameterize your npm calls, you now have your custom registry URL
> >> in
> >>>>>> the lockfile for every package it installed from there. This makes
> >> it
> >>>>>> unusable for others.
> >>>>>>
> >>>>>> The issue that ultimately drove us away from this concept was that
> >>>>>> locally cached packages were installed over linked modules, because
> >>> the
> >>>>>> package manager did not understand that they are linked.
> >>>>>> But because they were linked, the cached package contents were
> >> placed
> >>>> in
> >>>>>> my local development checkout of that linked module. That obviously
> >>>>>> caused all uncommitted changes to be deleted.
> >>>>>>
> >>>>>> Additionally, if you already have linked modules set up, but the
> >>>>>> lockfile says that a certain dependency is to be replaced, it will
> >>> just
> >>>>>> break your link or replace your linked code as soon as you `npm
> >>>> install`.
> >>>>>> We had so many issues with this, I'm sure I'm only remembering the
> >>> tip
> >>>>>> of the ice berg. Maybe all of this was fixed somehow, but I doubt
> >> it.
> >>>> At
> >>>>>> the time when I reported these issues, there was little interest in
> >>>>>> resolving them.
> >>>>>>
> >>>>>> However, I'm not unfamiliar with the lockfile-driven workflow as
> >> many
> >>>>>> OSS projects I contributed to use it. It is not uncommon to
> >>> completely
> >>>>>> wipe your node_modules and/or package-lock.json to rebuild it,
> >>> because
> >>>>>> of corruptions in either entity. And that is something that has
> >> been
> >>>>>> confirmed to me many times by other developers. As in "That's just
> >>> how
> >>>>>> it is."
> >>>>>>
> >>>>>> This entire area of issues was not exclusive to npm either. We
> >>>>>> extensively evaluated yarn regarding these aspects and it performed
> >>>> just
> >>>>>> as poorly.
> >>>>>>
> >>>>>> I consider these aspects unacceptable for a development workflow as
> >>>> they
> >>>>>> introduce an unreliability where I can't have one.
> >>>>>>
> >>>>>> If someone came out and told me "Hey, you've been doing it wrong
> >> all
> >>>>>> along. These are your mistakes and this is how you resolve them."
> >>> then
> >>>>>> I'd be very happy to hear that :)
> >>>>>>
> >>>>>> On 2018-09-14 15:13, raphinesse@gmail.com wrote:
> >>>>>>> Thanks for picking this up again Chris. I think now is a better
> >>> time
> >>>> for
> >>>>>>> second thoughts than later.
> >>>>>>>
> >>>>>>> I've had a look at your experiment and the behavior you observed
> >> is
> >>>> to be
> >>>>>>> expected and desirable, as I explained in more detail in a
> >> comment
> >>>> [1]
> >>>>>>> there. As I also mentioned there, deleting and regenerating
> >>>> package-lock.json
> >>>>>>> is a valid approach _if and when_ you want to do a full
> >> dependency
> >>>> update,
> >>>>>>> as we regularly do.
> >>>>>>>
> >>>>>>> Also, thanks for posting Oliver's message here for better
> >>> visibility
> >>>> in
> >>>>>>> this discussion. What I _do_ find a bit disturbing is the
> >> problems
> >>> he
> >>>>>>> mentioned regarding linking (as in `npm link`) of different
> >> modules
> >>>> which
> >>>>>>> are all using lock files. He expressed his concern regarding that
> >>>>>>> particular use-case again in a comment [2] of a PR where we
> >> touched
> >>>> that
> >>>>>>> topic. I think it is important we test this, since the ability to
> >>>> link
> >>>>>>> modules is vital for our development workflow and I have no
> >>>> experience with
> >>>>>>> package-lock.json in projects where a lot of linking is
> >> necessary.
> >>>>>>> Finally, I think we might need to re-evaluate our presumed
> >>> knowledge
> >>>> about
> >>>>>>> the topic at hand. I encourage all those interested to read
> >>>> [3][4][5] so we
> >>>>>>> all know what we are talking about. I had my facts wrong too and
> >>>> nobody
> >>>>>>> corrected me, when I uttered them here in this thread. So here's
> >> a
> >>>> quick
> >>>>>>> (probably incomplete) round up of what package-lock.json does and
> >>>> does not
> >>>>>>> do:
> >>>>>>>
> >>>>>>> - It does provide a snapshot of the dependency tree that can
> >> be
> >>>>>>> committed into source control to avoid automatic updates of
> >>>> (transitive)
> >>>>>>> dependencies break the build _during development and CI_
> >>>>>>> - It _does not_ ensure that a user installing the published
> >>>> package gets
> >>>>>>> exactly the dependency versions that are specified in
> >>>> package-lock.json.
> >>>>>>> That is what npm-shrinkwrap.json [5] is for.
> >>>>>>> - It does speed up installation of dependencies in
> >> conjunction
> >>>> with `npm
> >>>>>>> ci` by skipping the entire dependency resolution and using
> >> the
> >>>> versions
> >>>>>>> from the lock file.
> >>>>>>> - It is required to be present for `npm audit` [6], although
> >> it
> >>>> could be
> >>>>>>> generated ad-hoc.
> >>>>>>> - It is possible to manually tinker with the lock file to fix
> >>>> audit
> >>>>>>> issues with transitive dependencies that have no update
> >>>> available. This
> >>>>>>> requires some special care to prevent npm from resetting
> >> these
> >>>> manual
> >>>>>>> changes, but it's a valuable last-resort option. However,
> >> this
> >>>> is far more
> >>>>>>> useful with npm-shrinkwrap.json to create releases without
> >>>> security
> >>>>>>> issues.
> >>>>>>>
> >>>>>>> With that cleared up, my stance on committing package-lock.json
> >> is
> >>> as
> >>>>>>> follows:
> >>>>>>>
> >>>>>>> - Faster CI installations and faster/easier usage of `npm
> >>> audit`
> >>>> are
> >>>>>>> purely convenience features for me.
> >>>>>>> - Consistent developer builds and updates only on-demand are
> >>> real
> >>>>>>> advantages for me. I just recently spent hours finding out
> >> why
> >>>> some tests
> >>>>>>> of cordova-lib were suddenly failing. It turned out it was
> >>>> caused by an
> >>>>>>> update to `jasmine@3.2.0`.
> >>>>>>> - If the package-lock.json really should interfere with our
> >>>> ability to
> >>>>>>> link repositories for development, then that would be a deal
> >>>> breaker for me.
> >>>>>>> However, the primary goal that I wanted to achieve was _immutable
> >>>>>>> releases_. That is, installing e.g. `cordova@9` should _always
> >>>> install the
> >>>>>>> exact same set of packages_. What we need for that is
> >>>> npm-shrinkwrap.json.
> >>>>>>> So IMO whether we decide for or against using package-lock.json,
> >> we
> >>>> should
> >>>>>>> lock down the dependencies for releases of our CLIs, platforms
> >> and
> >>>> possibly
> >>>>>>> plugins by generating and committing a npm-shrinkwrap.json to the
> >>>> _release
> >>>>>>> branch_ before packaging the release.
> >>>>>>>
> >>>>>>> Cheers,
> >>>>>>> Raphael
> >>>>>>>
> >>>>>>> [1]:
> >>>> https://github.com/apache/cordova-cli/pull/325#issuecomment-421336025
> >>>>>>> [2]:
> >>>>>>>
> >> https://github.com/raphinesse/cordova-common/pull/1#issuecomment-420950433
> >>>>>>> [3]: https://docs.npmjs.com/files/package-locks
> >>>>>>> [4]: https://docs.npmjs.com/files/package-lock.json
> >>>>>>> [5]: https://docs.npmjs.com/files/shrinkwrap.json
> >>>>>>> [6]:
> >>> https://docs.npmjs.com/getting-started/running-a-security-audit
> >>>>>>
> >>>>>>
> >> ---------------------------------------------------------------------
> >>>>>> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> >>>>>> For additional commands, e-mail: dev-help@cordova.apache.org
> >>>>>>
> >>>> ---------------------------------------------------------------------
> >>>> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> >>>> For additional commands, e-mail: dev-help@cordova.apache.org
> >>>>
> >>>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> For additional commands, e-mail: dev-help@cordova.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
Re: [DISCUSS] When to do npm shrinkwrap?
Posted by Chris Brody <ch...@gmail.com>.
Raphael I did see your argument for considering shrinkwrap once before
in [1] (seems to be buried at the end of another discussion). But then
I saw the point *not* to turn package-lock.json into npm shrinkwrap in
[2] and maybe in [3]. So I was starting to get a bit lost here.
It would be great if you would have a chance to explain your proposal
in more detail, maybe walk us through more slowly. Sorry to be a pain
here.
[1]: https://lists.apache.org/thread.html/c06d35b6944fb44a04449a7baf10797f8ae11cbcfddb3e90851dc0b9@%3Cdev.cordova.apache.org%3E
[2]: https://lists.apache.org/thread.html/463315d120fae1ed78aa825f58b41b2cce0397dba58dfdfecc442d91@%3Cdev.cordova.apache.org%3E
[3]: https://lists.apache.org/thread.html/e336ac350b6208e6ff3e3b852d0be9f7cb88dceb522b2fe0390582bb@%3Cdev.cordova.apache.org%3E
On Mon, Sep 17, 2018 at 7:31 AM <ra...@gmail.com> wrote:
>
> I do not agree with your understanding here. We should use shrinkwrap for
> all releases. Please see my _very extensive_ argument and explanation on
> that topic in the thread you referenced [1].
>
> It has all kinds of references too, some of which you then found again
> yourself in a later message of the same thread [2]. I have to ask: did you
> read my message [1] at all?
>
> [1]:
> https://lists.apache.org/thread.html/c06d35b6944fb44a04449a7baf10797f8ae11cbcfddb3e90851dc0b9@%3Cdev.cordova.apache.org%3E
> [2]:
> https://lists.apache.org/thread.html/bce77e5892543003c3b3671cb6a2e9e02c33485073e691c6cf2c5fcf@%3Cdev.cordova.apache.org%3E
>
> Am Mo., 17. Sep. 2018 um 13:14 Uhr schrieb Chris Brody <
> chris.brody@gmail.com>:
>
> > Originated in another thread ([1]), moving into a new thread.
> >
> > My understanding is that we should only do npm shrinkwrap if needed to
> > deal with a dependency problem in an end-user package such as
> > cordova-cli.
> >
> > I would like to ask people to please use references whenever possible.
> >
> > [1]
> > https://lists.apache.org/thread.html/7f92561d382f143aaf49e083bbe215dcf95a3f4d8b6e3cbb6089a5f3@%3Cdev.cordova.apache.org%3E
> >
> > On Mon, Sep 17, 2018 at 5:38 AM Oliver Salzburg
> > <ol...@gmail.com> wrote:
> > >
> > > +1 to shrinkwrap before publish
> > >
> > > On 2018-09-15 00:27, raphinesse@gmail.com wrote:
> > > > Please note that I'm not suggesting to commit shrinkwrap to the master
> > of
> > > > every repository. That is what had been discussed in the mailing list
> > > > thread from 2014 that you shared. That option was dismissed then, and
> > > > rightly so. It is also strongly advised against in the npm docs.
> > > >
> > > > I suggest using it only in release branches of CLI or other non-library
> > > > releases, so they are immutable and don't mutate over time. _This is
> > > > exactly the intended use_. And it comes at no cost other than having
> > to run
> > > > `npm shrinkwrap` right before the release.
> > > >
> > > > Plus, it allows us to override transitive dependency versions for a
> > release
> > > > if absolutely necessary.
> > > >
> > > > Am Sa., 15. Sep. 2018 um 00:16 Uhr schrieb Jesse <
> > purplecabbage@gmail.com>:
> > > >
> > > >> I personally see shrinkwrap as a step in the wrong direction, I feel
> > like
> > > >> it has been explored in the past, and the general consensus is that
> > it is
> > > >> something to avoid.
> > > >> If committing package-lock is contentious, I am okay with skipping
> > it, but
> > > >> we need to be careful to lock down our own dependencies and not use
> > > >> wildcards.
> > > >>
> > > >> We have flexibility in our current GitHub repos, which allow us to
> > remix
> > > >> and freely link other dependent packages so we can debug right
> > through our
> > > >> deps. I agree we need more stability around the release/publish
> > steps so
> > > >> we can move more quickly through the collection of packages that make
> > up a
> > > >> 'tools' release.
> > > >>
> > > >> @purplecabbage
> > > >> risingj.com
> > > >>
> > > >>
> > > >> On Fri, Sep 14, 2018 at 2:56 PM <ra...@gmail.com> wrote:
> > > >>
> > > >>> No. This won't fix anything. Plus it goes directly against npm's
> > > >>> recommendation. Please double check the docs for the use cases of
> > > >>> package-lock.json vs npm-shrinkwrap.json.
> > > >>>
> > > >>> Am Fr., 14. Sep. 2018 um 23:48 Uhr schrieb Chris Brody <
> > > >>> chris.brody@gmail.com>:
> > > >>>
> > > >>>> A really nice alternative may be to turn the generated
> > > >>>> package-lock.json into npm-shrinkwrap.json (using npm shrinkwrap
> > > >>>> command) then commit npm-shrinkwrap.json. Then I think any other npm
> > > >>>> install updates would update npm-shrinkwrap.json instead of
> > > >>>> package-lock.json. Could be more predictable and easier to
> > understand.
> > > >>>>
> > > >>>> This was already discussed in 2014 [1], thanks to Jesse for the
> > link in
> > > >>>> [2].
> > > >>>>
> > > >>>> Thanks for the suggestion to use npm shrinkwrap as a solution for
> > > >>>> cordova-cli 8.1.0 minor release in [2].
> > > >>>>
> > > >>>> [1]
> > > >>>>
> > > >>
> > https://lists.apache.org/thread.html/99184622129935eb473e843e583bf6648faff279a014e8508cc2c660@1411013202@%3Cdev.cordova.apache.org%3E
> > > >>>> [2]
> > > >>>>
> > > >>
> > https://lists.apache.org/thread.html/f89a074add24f2ace7006b0211cf43a47cc5c1a0a65932fc22515828@%3Cdev.cordova.apache.org%3E
> > > >>>> On Fri, Sep 14, 2018 at 3:53 PM Chris Brody <ch...@gmail.com>
> > > >>> wrote:
> > > >>>>> To be honest I have pretty limited experience with package lock
> > file
> > > >>>>> and it is now starting to show. From Oliver's very unfortunate
> > > >>>>> experience I would conclude that this is something we should do
> > very
> > > >>>>> carefully and not just on a whim. Some things I can think of:
> > > >>>>>
> > > >>>>> * always use recent version of npm such as npm@6.4.1 to generate
> > or
> > > >>>>> update package-lock.json
> > > >>>>> * do not use. npm cache when generating or updating
> > > >> package-lock.json,
> > > >>>>> or use the npm cache with extreme care (also limited experience for
> > > >>>>> me)
> > > >>>>> * be extremely careful with assumptions; I think we should both
> > > >>>>> double-check the documentation and do our own experimentation,
> > like I
> > > >>>>> did in <https://github.com/apache/cordova-cli/pull/325> to
> > validate
> > > >> as
> > > >>>>> best we can
> > > >>>>> * semver package seems to be a major library package used by npm;
> > we
> > > >>>>> should both read the documentation and experiment, ideally with its
> > > >>>>> own test cases
> > > >>>>>
> > > >>>>> On Fri, Sep 14, 2018 at 9:47 AM Oliver Salzburg
> > > >>>>> <ol...@gmail.com> wrote:
> > > >>>>>> The problems that appear when you have linked dependencies is that
> > > >>> npm
> > > >>>>>> will pick them up as being bundled and mark them as such in the
> > > >>>>>> lockfile. *However* this behavior has changed in the past. At one
> > > >>> point
> > > >>>>>> this affected any direct dependency, at another point it "only"
> > > >>>> affected
> > > >>>>>> dependencies of dependencies.
> > > >>>>>>
> > > >>>>>> Either way, the result is:
> > > >>>>>>
> > > >>>>>> a) a corrupted lockfile, which has dependencies marked as bundled
> > > >>>>>> b) a lockfile that lists incorrect versions, resulting from
> > linking
> > > >>>>>> temporary development snapshots together
> > > >>>>>>
> > > >>>>>> When you use a local npm cache and you neglected to correctly
> > > >>>>>> parameterize your npm calls, you now have your custom registry URL
> > > >> in
> > > >>>>>> the lockfile for every package it installed from there. This makes
> > > >> it
> > > >>>>>> unusable for others.
> > > >>>>>>
> > > >>>>>> The issue that ultimately drove us away from this concept was that
> > > >>>>>> locally cached packages were installed over linked modules,
> > because
> > > >>> the
> > > >>>>>> package manager did not understand that they are linked.
> > > >>>>>> But because they were linked, the cached package contents were
> > > >> placed
> > > >>>> in
> > > >>>>>> my local development checkout of that linked module. That
> > obviously
> > > >>>>>> caused all uncommitted changes to be deleted.
> > > >>>>>>
> > > >>>>>> Additionally, if you already have linked modules set up, but the
> > > >>>>>> lockfile says that a certain dependency is to be replaced, it will
> > > >>> just
> > > >>>>>> break your link or replace your linked code as soon as you `npm
> > > >>>> install`.
> > > >>>>>> We had so many issues with this, I'm sure I'm only remembering the
> > > >>> tip
> > > >>>>>> of the ice berg. Maybe all of this was fixed somehow, but I doubt
> > > >> it.
> > > >>>> At
> > > >>>>>> the time when I reported these issues, there was little interest
> > in
> > > >>>>>> resolving them.
> > > >>>>>>
> > > >>>>>> However, I'm not unfamiliar with the lockfile-driven workflow as
> > > >> many
> > > >>>>>> OSS projects I contributed to use it. It is not uncommon to
> > > >>> completely
> > > >>>>>> wipe your node_modules and/or package-lock.json to rebuild it,
> > > >>> because
> > > >>>>>> of corruptions in either entity. And that is something that has
> > > >> been
> > > >>>>>> confirmed to me many times by other developers. As in "That's just
> > > >>> how
> > > >>>>>> it is."
> > > >>>>>>
> > > >>>>>> This entire area of issues was not exclusive to npm either. We
> > > >>>>>> extensively evaluated yarn regarding these aspects and it
> > performed
> > > >>>> just
> > > >>>>>> as poorly.
> > > >>>>>>
> > > >>>>>> I consider these aspects unacceptable for a development workflow
> > as
> > > >>>> they
> > > >>>>>> introduce an unreliability where I can't have one.
> > > >>>>>>
> > > >>>>>> If someone came out and told me "Hey, you've been doing it wrong
> > > >> all
> > > >>>>>> along. These are your mistakes and this is how you resolve them."
> > > >>> then
> > > >>>>>> I'd be very happy to hear that :)
> > > >>>>>>
> > > >>>>>> On 2018-09-14 15:13, raphinesse@gmail.com wrote:
> > > >>>>>>> Thanks for picking this up again Chris. I think now is a better
> > > >>> time
> > > >>>> for
> > > >>>>>>> second thoughts than later.
> > > >>>>>>>
> > > >>>>>>> I've had a look at your experiment and the behavior you observed
> > > >> is
> > > >>>> to be
> > > >>>>>>> expected and desirable, as I explained in more detail in a
> > > >> comment
> > > >>>> [1]
> > > >>>>>>> there. As I also mentioned there, deleting and regenerating
> > > >>>> package-lock.json
> > > >>>>>>> is a valid approach _if and when_ you want to do a full
> > > >> dependency
> > > >>>> update,
> > > >>>>>>> as we regularly do.
> > > >>>>>>>
> > > >>>>>>> Also, thanks for posting Oliver's message here for better
> > > >>> visibility
> > > >>>> in
> > > >>>>>>> this discussion. What I _do_ find a bit disturbing is the
> > > >> problems
> > > >>> he
> > > >>>>>>> mentioned regarding linking (as in `npm link`) of different
> > > >> modules
> > > >>>> which
> > > >>>>>>> are all using lock files. He expressed his concern regarding that
> > > >>>>>>> particular use-case again in a comment [2] of a PR where we
> > > >> touched
> > > >>>> that
> > > >>>>>>> topic. I think it is important we test this, since the ability to
> > > >>>> link
> > > >>>>>>> modules is vital for our development workflow and I have no
> > > >>>> experience with
> > > >>>>>>> package-lock.json in projects where a lot of linking is
> > > >> necessary.
> > > >>>>>>> Finally, I think we might need to re-evaluate our presumed
> > > >>> knowledge
> > > >>>> about
> > > >>>>>>> the topic at hand. I encourage all those interested to read
> > > >>>> [3][4][5] so we
> > > >>>>>>> all know what we are talking about. I had my facts wrong too and
> > > >>>> nobody
> > > >>>>>>> corrected me, when I uttered them here in this thread. So here's
> > > >> a
> > > >>>> quick
> > > >>>>>>> (probably incomplete) round up of what package-lock.json does and
> > > >>>> does not
> > > >>>>>>> do:
> > > >>>>>>>
> > > >>>>>>> - It does provide a snapshot of the dependency tree that can
> > > >> be
> > > >>>>>>> committed into source control to avoid automatic updates of
> > > >>>> (transitive)
> > > >>>>>>> dependencies break the build _during development and CI_
> > > >>>>>>> - It _does not_ ensure that a user installing the published
> > > >>>> package gets
> > > >>>>>>> exactly the dependency versions that are specified in
> > > >>>> package-lock.json.
> > > >>>>>>> That is what npm-shrinkwrap.json [5] is for.
> > > >>>>>>> - It does speed up installation of dependencies in
> > > >> conjunction
> > > >>>> with `npm
> > > >>>>>>> ci` by skipping the entire dependency resolution and using
> > > >> the
> > > >>>> versions
> > > >>>>>>> from the lock file.
> > > >>>>>>> - It is required to be present for `npm audit` [6], although
> > > >> it
> > > >>>> could be
> > > >>>>>>> generated ad-hoc.
> > > >>>>>>> - It is possible to manually tinker with the lock file to
> > fix
> > > >>>> audit
> > > >>>>>>> issues with transitive dependencies that have no update
> > > >>>> available. This
> > > >>>>>>> requires some special care to prevent npm from resetting
> > > >> these
> > > >>>> manual
> > > >>>>>>> changes, but it's a valuable last-resort option. However,
> > > >> this
> > > >>>> is far more
> > > >>>>>>> useful with npm-shrinkwrap.json to create releases without
> > > >>>> security
> > > >>>>>>> issues.
> > > >>>>>>>
> > > >>>>>>> With that cleared up, my stance on committing package-lock.json
> > > >> is
> > > >>> as
> > > >>>>>>> follows:
> > > >>>>>>>
> > > >>>>>>> - Faster CI installations and faster/easier usage of `npm
> > > >>> audit`
> > > >>>> are
> > > >>>>>>> purely convenience features for me.
> > > >>>>>>> - Consistent developer builds and updates only on-demand are
> > > >>> real
> > > >>>>>>> advantages for me. I just recently spent hours finding out
> > > >> why
> > > >>>> some tests
> > > >>>>>>> of cordova-lib were suddenly failing. It turned out it was
> > > >>>> caused by an
> > > >>>>>>> update to `jasmine@3.2.0`.
> > > >>>>>>> - If the package-lock.json really should interfere with our
> > > >>>> ability to
> > > >>>>>>> link repositories for development, then that would be a deal
> > > >>>> breaker for me.
> > > >>>>>>> However, the primary goal that I wanted to achieve was _immutable
> > > >>>>>>> releases_. That is, installing e.g. `cordova@9` should _always
> > > >>>> install the
> > > >>>>>>> exact same set of packages_. What we need for that is
> > > >>>> npm-shrinkwrap.json.
> > > >>>>>>> So IMO whether we decide for or against using package-lock.json,
> > > >> we
> > > >>>> should
> > > >>>>>>> lock down the dependencies for releases of our CLIs, platforms
> > > >> and
> > > >>>> possibly
> > > >>>>>>> plugins by generating and committing a npm-shrinkwrap.json to the
> > > >>>> _release
> > > >>>>>>> branch_ before packaging the release.
> > > >>>>>>>
> > > >>>>>>> Cheers,
> > > >>>>>>> Raphael
> > > >>>>>>>
> > > >>>>>>> [1]:
> > > >>>>
> > https://github.com/apache/cordova-cli/pull/325#issuecomment-421336025
> > > >>>>>>> [2]:
> > > >>>>>>>
> > > >>
> > https://github.com/raphinesse/cordova-common/pull/1#issuecomment-420950433
> > > >>>>>>> [3]: https://docs.npmjs.com/files/package-locks
> > > >>>>>>> [4]: https://docs.npmjs.com/files/package-lock.json
> > > >>>>>>> [5]: https://docs.npmjs.com/files/shrinkwrap.json
> > > >>>>>>> [6]:
> > > >>> https://docs.npmjs.com/getting-started/running-a-security-audit
> > > >>>>>>
> > > >>>>>>
> > > >> ---------------------------------------------------------------------
> > > >>>>>> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> > > >>>>>> For additional commands, e-mail: dev-help@cordova.apache.org
> > > >>>>>>
> > > >>>>
> > ---------------------------------------------------------------------
> > > >>>> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> > > >>>> For additional commands, e-mail: dev-help@cordova.apache.org
> > > >>>>
> > > >>>>
> > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> > > For additional commands, e-mail: dev-help@cordova.apache.org
> > >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> > For additional commands, e-mail: dev-help@cordova.apache.org
> >
> >
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
Re: [DISCUSS] When to do npm shrinkwrap?
Posted by ra...@gmail.com.
I do not agree with your understanding here. We should use shrinkwrap for
all releases. Please see my _very extensive_ argument and explanation on
that topic in the thread you referenced [1].
It has all kinds of references too, some of which you then found again
yourself in a later message of the same thread [2]. I have to ask: did you
read my message [1] at all?
[1]:
https://lists.apache.org/thread.html/c06d35b6944fb44a04449a7baf10797f8ae11cbcfddb3e90851dc0b9@%3Cdev.cordova.apache.org%3E
[2]:
https://lists.apache.org/thread.html/bce77e5892543003c3b3671cb6a2e9e02c33485073e691c6cf2c5fcf@%3Cdev.cordova.apache.org%3E
Am Mo., 17. Sep. 2018 um 13:14 Uhr schrieb Chris Brody <
chris.brody@gmail.com>:
> Originated in another thread ([1]), moving into a new thread.
>
> My understanding is that we should only do npm shrinkwrap if needed to
> deal with a dependency problem in an end-user package such as
> cordova-cli.
>
> I would like to ask people to please use references whenever possible.
>
> [1]
> https://lists.apache.org/thread.html/7f92561d382f143aaf49e083bbe215dcf95a3f4d8b6e3cbb6089a5f3@%3Cdev.cordova.apache.org%3E
>
> On Mon, Sep 17, 2018 at 5:38 AM Oliver Salzburg
> <ol...@gmail.com> wrote:
> >
> > +1 to shrinkwrap before publish
> >
> > On 2018-09-15 00:27, raphinesse@gmail.com wrote:
> > > Please note that I'm not suggesting to commit shrinkwrap to the master
> of
> > > every repository. That is what had been discussed in the mailing list
> > > thread from 2014 that you shared. That option was dismissed then, and
> > > rightly so. It is also strongly advised against in the npm docs.
> > >
> > > I suggest using it only in release branches of CLI or other non-library
> > > releases, so they are immutable and don't mutate over time. _This is
> > > exactly the intended use_. And it comes at no cost other than having
> to run
> > > `npm shrinkwrap` right before the release.
> > >
> > > Plus, it allows us to override transitive dependency versions for a
> release
> > > if absolutely necessary.
> > >
> > > Am Sa., 15. Sep. 2018 um 00:16 Uhr schrieb Jesse <
> purplecabbage@gmail.com>:
> > >
> > >> I personally see shrinkwrap as a step in the wrong direction, I feel
> like
> > >> it has been explored in the past, and the general consensus is that
> it is
> > >> something to avoid.
> > >> If committing package-lock is contentious, I am okay with skipping
> it, but
> > >> we need to be careful to lock down our own dependencies and not use
> > >> wildcards.
> > >>
> > >> We have flexibility in our current GitHub repos, which allow us to
> remix
> > >> and freely link other dependent packages so we can debug right
> through our
> > >> deps. I agree we need more stability around the release/publish
> steps so
> > >> we can move more quickly through the collection of packages that make
> up a
> > >> 'tools' release.
> > >>
> > >> @purplecabbage
> > >> risingj.com
> > >>
> > >>
> > >> On Fri, Sep 14, 2018 at 2:56 PM <ra...@gmail.com> wrote:
> > >>
> > >>> No. This won't fix anything. Plus it goes directly against npm's
> > >>> recommendation. Please double check the docs for the use cases of
> > >>> package-lock.json vs npm-shrinkwrap.json.
> > >>>
> > >>> Am Fr., 14. Sep. 2018 um 23:48 Uhr schrieb Chris Brody <
> > >>> chris.brody@gmail.com>:
> > >>>
> > >>>> A really nice alternative may be to turn the generated
> > >>>> package-lock.json into npm-shrinkwrap.json (using npm shrinkwrap
> > >>>> command) then commit npm-shrinkwrap.json. Then I think any other npm
> > >>>> install updates would update npm-shrinkwrap.json instead of
> > >>>> package-lock.json. Could be more predictable and easier to
> understand.
> > >>>>
> > >>>> This was already discussed in 2014 [1], thanks to Jesse for the
> link in
> > >>>> [2].
> > >>>>
> > >>>> Thanks for the suggestion to use npm shrinkwrap as a solution for
> > >>>> cordova-cli 8.1.0 minor release in [2].
> > >>>>
> > >>>> [1]
> > >>>>
> > >>
> https://lists.apache.org/thread.html/99184622129935eb473e843e583bf6648faff279a014e8508cc2c660@1411013202@%3Cdev.cordova.apache.org%3E
> > >>>> [2]
> > >>>>
> > >>
> https://lists.apache.org/thread.html/f89a074add24f2ace7006b0211cf43a47cc5c1a0a65932fc22515828@%3Cdev.cordova.apache.org%3E
> > >>>> On Fri, Sep 14, 2018 at 3:53 PM Chris Brody <ch...@gmail.com>
> > >>> wrote:
> > >>>>> To be honest I have pretty limited experience with package lock
> file
> > >>>>> and it is now starting to show. From Oliver's very unfortunate
> > >>>>> experience I would conclude that this is something we should do
> very
> > >>>>> carefully and not just on a whim. Some things I can think of:
> > >>>>>
> > >>>>> * always use recent version of npm such as npm@6.4.1 to generate
> or
> > >>>>> update package-lock.json
> > >>>>> * do not use. npm cache when generating or updating
> > >> package-lock.json,
> > >>>>> or use the npm cache with extreme care (also limited experience for
> > >>>>> me)
> > >>>>> * be extremely careful with assumptions; I think we should both
> > >>>>> double-check the documentation and do our own experimentation,
> like I
> > >>>>> did in <https://github.com/apache/cordova-cli/pull/325> to
> validate
> > >> as
> > >>>>> best we can
> > >>>>> * semver package seems to be a major library package used by npm;
> we
> > >>>>> should both read the documentation and experiment, ideally with its
> > >>>>> own test cases
> > >>>>>
> > >>>>> On Fri, Sep 14, 2018 at 9:47 AM Oliver Salzburg
> > >>>>> <ol...@gmail.com> wrote:
> > >>>>>> The problems that appear when you have linked dependencies is that
> > >>> npm
> > >>>>>> will pick them up as being bundled and mark them as such in the
> > >>>>>> lockfile. *However* this behavior has changed in the past. At one
> > >>> point
> > >>>>>> this affected any direct dependency, at another point it "only"
> > >>>> affected
> > >>>>>> dependencies of dependencies.
> > >>>>>>
> > >>>>>> Either way, the result is:
> > >>>>>>
> > >>>>>> a) a corrupted lockfile, which has dependencies marked as bundled
> > >>>>>> b) a lockfile that lists incorrect versions, resulting from
> linking
> > >>>>>> temporary development snapshots together
> > >>>>>>
> > >>>>>> When you use a local npm cache and you neglected to correctly
> > >>>>>> parameterize your npm calls, you now have your custom registry URL
> > >> in
> > >>>>>> the lockfile for every package it installed from there. This makes
> > >> it
> > >>>>>> unusable for others.
> > >>>>>>
> > >>>>>> The issue that ultimately drove us away from this concept was that
> > >>>>>> locally cached packages were installed over linked modules,
> because
> > >>> the
> > >>>>>> package manager did not understand that they are linked.
> > >>>>>> But because they were linked, the cached package contents were
> > >> placed
> > >>>> in
> > >>>>>> my local development checkout of that linked module. That
> obviously
> > >>>>>> caused all uncommitted changes to be deleted.
> > >>>>>>
> > >>>>>> Additionally, if you already have linked modules set up, but the
> > >>>>>> lockfile says that a certain dependency is to be replaced, it will
> > >>> just
> > >>>>>> break your link or replace your linked code as soon as you `npm
> > >>>> install`.
> > >>>>>> We had so many issues with this, I'm sure I'm only remembering the
> > >>> tip
> > >>>>>> of the ice berg. Maybe all of this was fixed somehow, but I doubt
> > >> it.
> > >>>> At
> > >>>>>> the time when I reported these issues, there was little interest
> in
> > >>>>>> resolving them.
> > >>>>>>
> > >>>>>> However, I'm not unfamiliar with the lockfile-driven workflow as
> > >> many
> > >>>>>> OSS projects I contributed to use it. It is not uncommon to
> > >>> completely
> > >>>>>> wipe your node_modules and/or package-lock.json to rebuild it,
> > >>> because
> > >>>>>> of corruptions in either entity. And that is something that has
> > >> been
> > >>>>>> confirmed to me many times by other developers. As in "That's just
> > >>> how
> > >>>>>> it is."
> > >>>>>>
> > >>>>>> This entire area of issues was not exclusive to npm either. We
> > >>>>>> extensively evaluated yarn regarding these aspects and it
> performed
> > >>>> just
> > >>>>>> as poorly.
> > >>>>>>
> > >>>>>> I consider these aspects unacceptable for a development workflow
> as
> > >>>> they
> > >>>>>> introduce an unreliability where I can't have one.
> > >>>>>>
> > >>>>>> If someone came out and told me "Hey, you've been doing it wrong
> > >> all
> > >>>>>> along. These are your mistakes and this is how you resolve them."
> > >>> then
> > >>>>>> I'd be very happy to hear that :)
> > >>>>>>
> > >>>>>> On 2018-09-14 15:13, raphinesse@gmail.com wrote:
> > >>>>>>> Thanks for picking this up again Chris. I think now is a better
> > >>> time
> > >>>> for
> > >>>>>>> second thoughts than later.
> > >>>>>>>
> > >>>>>>> I've had a look at your experiment and the behavior you observed
> > >> is
> > >>>> to be
> > >>>>>>> expected and desirable, as I explained in more detail in a
> > >> comment
> > >>>> [1]
> > >>>>>>> there. As I also mentioned there, deleting and regenerating
> > >>>> package-lock.json
> > >>>>>>> is a valid approach _if and when_ you want to do a full
> > >> dependency
> > >>>> update,
> > >>>>>>> as we regularly do.
> > >>>>>>>
> > >>>>>>> Also, thanks for posting Oliver's message here for better
> > >>> visibility
> > >>>> in
> > >>>>>>> this discussion. What I _do_ find a bit disturbing is the
> > >> problems
> > >>> he
> > >>>>>>> mentioned regarding linking (as in `npm link`) of different
> > >> modules
> > >>>> which
> > >>>>>>> are all using lock files. He expressed his concern regarding that
> > >>>>>>> particular use-case again in a comment [2] of a PR where we
> > >> touched
> > >>>> that
> > >>>>>>> topic. I think it is important we test this, since the ability to
> > >>>> link
> > >>>>>>> modules is vital for our development workflow and I have no
> > >>>> experience with
> > >>>>>>> package-lock.json in projects where a lot of linking is
> > >> necessary.
> > >>>>>>> Finally, I think we might need to re-evaluate our presumed
> > >>> knowledge
> > >>>> about
> > >>>>>>> the topic at hand. I encourage all those interested to read
> > >>>> [3][4][5] so we
> > >>>>>>> all know what we are talking about. I had my facts wrong too and
> > >>>> nobody
> > >>>>>>> corrected me, when I uttered them here in this thread. So here's
> > >> a
> > >>>> quick
> > >>>>>>> (probably incomplete) round up of what package-lock.json does and
> > >>>> does not
> > >>>>>>> do:
> > >>>>>>>
> > >>>>>>> - It does provide a snapshot of the dependency tree that can
> > >> be
> > >>>>>>> committed into source control to avoid automatic updates of
> > >>>> (transitive)
> > >>>>>>> dependencies break the build _during development and CI_
> > >>>>>>> - It _does not_ ensure that a user installing the published
> > >>>> package gets
> > >>>>>>> exactly the dependency versions that are specified in
> > >>>> package-lock.json.
> > >>>>>>> That is what npm-shrinkwrap.json [5] is for.
> > >>>>>>> - It does speed up installation of dependencies in
> > >> conjunction
> > >>>> with `npm
> > >>>>>>> ci` by skipping the entire dependency resolution and using
> > >> the
> > >>>> versions
> > >>>>>>> from the lock file.
> > >>>>>>> - It is required to be present for `npm audit` [6], although
> > >> it
> > >>>> could be
> > >>>>>>> generated ad-hoc.
> > >>>>>>> - It is possible to manually tinker with the lock file to
> fix
> > >>>> audit
> > >>>>>>> issues with transitive dependencies that have no update
> > >>>> available. This
> > >>>>>>> requires some special care to prevent npm from resetting
> > >> these
> > >>>> manual
> > >>>>>>> changes, but it's a valuable last-resort option. However,
> > >> this
> > >>>> is far more
> > >>>>>>> useful with npm-shrinkwrap.json to create releases without
> > >>>> security
> > >>>>>>> issues.
> > >>>>>>>
> > >>>>>>> With that cleared up, my stance on committing package-lock.json
> > >> is
> > >>> as
> > >>>>>>> follows:
> > >>>>>>>
> > >>>>>>> - Faster CI installations and faster/easier usage of `npm
> > >>> audit`
> > >>>> are
> > >>>>>>> purely convenience features for me.
> > >>>>>>> - Consistent developer builds and updates only on-demand are
> > >>> real
> > >>>>>>> advantages for me. I just recently spent hours finding out
> > >> why
> > >>>> some tests
> > >>>>>>> of cordova-lib were suddenly failing. It turned out it was
> > >>>> caused by an
> > >>>>>>> update to `jasmine@3.2.0`.
> > >>>>>>> - If the package-lock.json really should interfere with our
> > >>>> ability to
> > >>>>>>> link repositories for development, then that would be a deal
> > >>>> breaker for me.
> > >>>>>>> However, the primary goal that I wanted to achieve was _immutable
> > >>>>>>> releases_. That is, installing e.g. `cordova@9` should _always
> > >>>> install the
> > >>>>>>> exact same set of packages_. What we need for that is
> > >>>> npm-shrinkwrap.json.
> > >>>>>>> So IMO whether we decide for or against using package-lock.json,
> > >> we
> > >>>> should
> > >>>>>>> lock down the dependencies for releases of our CLIs, platforms
> > >> and
> > >>>> possibly
> > >>>>>>> plugins by generating and committing a npm-shrinkwrap.json to the
> > >>>> _release
> > >>>>>>> branch_ before packaging the release.
> > >>>>>>>
> > >>>>>>> Cheers,
> > >>>>>>> Raphael
> > >>>>>>>
> > >>>>>>> [1]:
> > >>>>
> https://github.com/apache/cordova-cli/pull/325#issuecomment-421336025
> > >>>>>>> [2]:
> > >>>>>>>
> > >>
> https://github.com/raphinesse/cordova-common/pull/1#issuecomment-420950433
> > >>>>>>> [3]: https://docs.npmjs.com/files/package-locks
> > >>>>>>> [4]: https://docs.npmjs.com/files/package-lock.json
> > >>>>>>> [5]: https://docs.npmjs.com/files/shrinkwrap.json
> > >>>>>>> [6]:
> > >>> https://docs.npmjs.com/getting-started/running-a-security-audit
> > >>>>>>
> > >>>>>>
> > >> ---------------------------------------------------------------------
> > >>>>>> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> > >>>>>> For additional commands, e-mail: dev-help@cordova.apache.org
> > >>>>>>
> > >>>>
> ---------------------------------------------------------------------
> > >>>> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> > >>>> For additional commands, e-mail: dev-help@cordova.apache.org
> > >>>>
> > >>>>
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> > For additional commands, e-mail: dev-help@cordova.apache.org
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> For additional commands, e-mail: dev-help@cordova.apache.org
>
>
Re: [DISCUSS] When to do npm shrinkwrap?
Posted by Oliver Salzburg <ol...@gmail.com>.
The shrinkwrap *can* be a tool to have more control over the dependency
tree. But I would consider that a hack. To my understanding the
shrinkwrap is not meant to be manually edited.
In general, it is just a best practice means to lock down dependency
versions, without the negative effects of maintaining a lockfile during
development.
However, shrinkwrapping produces the same possibly erroneous output that
the lockfile has. So this (for me) is something that absolutely requires
an isolated environment. Linked modules are detected as being bundled
dependencies for example. So, contrary to what npm says, this should not
be committed to VCS either :)
On 2018-09-17 13:14, Chris Brody wrote:
> Originated in another thread ([1]), moving into a new thread.
>
> My understanding is that we should only do npm shrinkwrap if needed to
> deal with a dependency problem in an end-user package such as
> cordova-cli.
>
> I would like to ask people to please use references whenever possible.
>
> [1] https://lists.apache.org/thread.html/7f92561d382f143aaf49e083bbe215dcf95a3f4d8b6e3cbb6089a5f3@%3Cdev.cordova.apache.org%3E
>
> On Mon, Sep 17, 2018 at 5:38 AM Oliver Salzburg
> <ol...@gmail.com> wrote:
>> +1 to shrinkwrap before publish
>>
>> On 2018-09-15 00:27, raphinesse@gmail.com wrote:
>>> Please note that I'm not suggesting to commit shrinkwrap to the master of
>>> every repository. That is what had been discussed in the mailing list
>>> thread from 2014 that you shared. That option was dismissed then, and
>>> rightly so. It is also strongly advised against in the npm docs.
>>>
>>> I suggest using it only in release branches of CLI or other non-library
>>> releases, so they are immutable and don't mutate over time. _This is
>>> exactly the intended use_. And it comes at no cost other than having to run
>>> `npm shrinkwrap` right before the release.
>>>
>>> Plus, it allows us to override transitive dependency versions for a release
>>> if absolutely necessary.
>>>
>>> Am Sa., 15. Sep. 2018 um 00:16 Uhr schrieb Jesse <pu...@gmail.com>:
>>>
>>>> I personally see shrinkwrap as a step in the wrong direction, I feel like
>>>> it has been explored in the past, and the general consensus is that it is
>>>> something to avoid.
>>>> If committing package-lock is contentious, I am okay with skipping it, but
>>>> we need to be careful to lock down our own dependencies and not use
>>>> wildcards.
>>>>
>>>> We have flexibility in our current GitHub repos, which allow us to remix
>>>> and freely link other dependent packages so we can debug right through our
>>>> deps. I agree we need more stability around the release/publish steps so
>>>> we can move more quickly through the collection of packages that make up a
>>>> 'tools' release.
>>>>
>>>> @purplecabbage
>>>> risingj.com
>>>>
>>>>
>>>> On Fri, Sep 14, 2018 at 2:56 PM <ra...@gmail.com> wrote:
>>>>
>>>>> No. This won't fix anything. Plus it goes directly against npm's
>>>>> recommendation. Please double check the docs for the use cases of
>>>>> package-lock.json vs npm-shrinkwrap.json.
>>>>>
>>>>> Am Fr., 14. Sep. 2018 um 23:48 Uhr schrieb Chris Brody <
>>>>> chris.brody@gmail.com>:
>>>>>
>>>>>> A really nice alternative may be to turn the generated
>>>>>> package-lock.json into npm-shrinkwrap.json (using npm shrinkwrap
>>>>>> command) then commit npm-shrinkwrap.json. Then I think any other npm
>>>>>> install updates would update npm-shrinkwrap.json instead of
>>>>>> package-lock.json. Could be more predictable and easier to understand.
>>>>>>
>>>>>> This was already discussed in 2014 [1], thanks to Jesse for the link in
>>>>>> [2].
>>>>>>
>>>>>> Thanks for the suggestion to use npm shrinkwrap as a solution for
>>>>>> cordova-cli 8.1.0 minor release in [2].
>>>>>>
>>>>>> [1]
>>>>>>
>>>> https://lists.apache.org/thread.html/99184622129935eb473e843e583bf6648faff279a014e8508cc2c660@1411013202@%3Cdev.cordova.apache.org%3E
>>>>>> [2]
>>>>>>
>>>> https://lists.apache.org/thread.html/f89a074add24f2ace7006b0211cf43a47cc5c1a0a65932fc22515828@%3Cdev.cordova.apache.org%3E
>>>>>> On Fri, Sep 14, 2018 at 3:53 PM Chris Brody <ch...@gmail.com>
>>>>> wrote:
>>>>>>> To be honest I have pretty limited experience with package lock file
>>>>>>> and it is now starting to show. From Oliver's very unfortunate
>>>>>>> experience I would conclude that this is something we should do very
>>>>>>> carefully and not just on a whim. Some things I can think of:
>>>>>>>
>>>>>>> * always use recent version of npm such as npm@6.4.1 to generate or
>>>>>>> update package-lock.json
>>>>>>> * do not use. npm cache when generating or updating
>>>> package-lock.json,
>>>>>>> or use the npm cache with extreme care (also limited experience for
>>>>>>> me)
>>>>>>> * be extremely careful with assumptions; I think we should both
>>>>>>> double-check the documentation and do our own experimentation, like I
>>>>>>> did in <https://github.com/apache/cordova-cli/pull/325> to validate
>>>> as
>>>>>>> best we can
>>>>>>> * semver package seems to be a major library package used by npm; we
>>>>>>> should both read the documentation and experiment, ideally with its
>>>>>>> own test cases
>>>>>>>
>>>>>>> On Fri, Sep 14, 2018 at 9:47 AM Oliver Salzburg
>>>>>>> <ol...@gmail.com> wrote:
>>>>>>>> The problems that appear when you have linked dependencies is that
>>>>> npm
>>>>>>>> will pick them up as being bundled and mark them as such in the
>>>>>>>> lockfile. *However* this behavior has changed in the past. At one
>>>>> point
>>>>>>>> this affected any direct dependency, at another point it "only"
>>>>>> affected
>>>>>>>> dependencies of dependencies.
>>>>>>>>
>>>>>>>> Either way, the result is:
>>>>>>>>
>>>>>>>> a) a corrupted lockfile, which has dependencies marked as bundled
>>>>>>>> b) a lockfile that lists incorrect versions, resulting from linking
>>>>>>>> temporary development snapshots together
>>>>>>>>
>>>>>>>> When you use a local npm cache and you neglected to correctly
>>>>>>>> parameterize your npm calls, you now have your custom registry URL
>>>> in
>>>>>>>> the lockfile for every package it installed from there. This makes
>>>> it
>>>>>>>> unusable for others.
>>>>>>>>
>>>>>>>> The issue that ultimately drove us away from this concept was that
>>>>>>>> locally cached packages were installed over linked modules, because
>>>>> the
>>>>>>>> package manager did not understand that they are linked.
>>>>>>>> But because they were linked, the cached package contents were
>>>> placed
>>>>>> in
>>>>>>>> my local development checkout of that linked module. That obviously
>>>>>>>> caused all uncommitted changes to be deleted.
>>>>>>>>
>>>>>>>> Additionally, if you already have linked modules set up, but the
>>>>>>>> lockfile says that a certain dependency is to be replaced, it will
>>>>> just
>>>>>>>> break your link or replace your linked code as soon as you `npm
>>>>>> install`.
>>>>>>>> We had so many issues with this, I'm sure I'm only remembering the
>>>>> tip
>>>>>>>> of the ice berg. Maybe all of this was fixed somehow, but I doubt
>>>> it.
>>>>>> At
>>>>>>>> the time when I reported these issues, there was little interest in
>>>>>>>> resolving them.
>>>>>>>>
>>>>>>>> However, I'm not unfamiliar with the lockfile-driven workflow as
>>>> many
>>>>>>>> OSS projects I contributed to use it. It is not uncommon to
>>>>> completely
>>>>>>>> wipe your node_modules and/or package-lock.json to rebuild it,
>>>>> because
>>>>>>>> of corruptions in either entity. And that is something that has
>>>> been
>>>>>>>> confirmed to me many times by other developers. As in "That's just
>>>>> how
>>>>>>>> it is."
>>>>>>>>
>>>>>>>> This entire area of issues was not exclusive to npm either. We
>>>>>>>> extensively evaluated yarn regarding these aspects and it performed
>>>>>> just
>>>>>>>> as poorly.
>>>>>>>>
>>>>>>>> I consider these aspects unacceptable for a development workflow as
>>>>>> they
>>>>>>>> introduce an unreliability where I can't have one.
>>>>>>>>
>>>>>>>> If someone came out and told me "Hey, you've been doing it wrong
>>>> all
>>>>>>>> along. These are your mistakes and this is how you resolve them."
>>>>> then
>>>>>>>> I'd be very happy to hear that :)
>>>>>>>>
>>>>>>>> On 2018-09-14 15:13, raphinesse@gmail.com wrote:
>>>>>>>>> Thanks for picking this up again Chris. I think now is a better
>>>>> time
>>>>>> for
>>>>>>>>> second thoughts than later.
>>>>>>>>>
>>>>>>>>> I've had a look at your experiment and the behavior you observed
>>>> is
>>>>>> to be
>>>>>>>>> expected and desirable, as I explained in more detail in a
>>>> comment
>>>>>> [1]
>>>>>>>>> there. As I also mentioned there, deleting and regenerating
>>>>>> package-lock.json
>>>>>>>>> is a valid approach _if and when_ you want to do a full
>>>> dependency
>>>>>> update,
>>>>>>>>> as we regularly do.
>>>>>>>>>
>>>>>>>>> Also, thanks for posting Oliver's message here for better
>>>>> visibility
>>>>>> in
>>>>>>>>> this discussion. What I _do_ find a bit disturbing is the
>>>> problems
>>>>> he
>>>>>>>>> mentioned regarding linking (as in `npm link`) of different
>>>> modules
>>>>>> which
>>>>>>>>> are all using lock files. He expressed his concern regarding that
>>>>>>>>> particular use-case again in a comment [2] of a PR where we
>>>> touched
>>>>>> that
>>>>>>>>> topic. I think it is important we test this, since the ability to
>>>>>> link
>>>>>>>>> modules is vital for our development workflow and I have no
>>>>>> experience with
>>>>>>>>> package-lock.json in projects where a lot of linking is
>>>> necessary.
>>>>>>>>> Finally, I think we might need to re-evaluate our presumed
>>>>> knowledge
>>>>>> about
>>>>>>>>> the topic at hand. I encourage all those interested to read
>>>>>> [3][4][5] so we
>>>>>>>>> all know what we are talking about. I had my facts wrong too and
>>>>>> nobody
>>>>>>>>> corrected me, when I uttered them here in this thread. So here's
>>>> a
>>>>>> quick
>>>>>>>>> (probably incomplete) round up of what package-lock.json does and
>>>>>> does not
>>>>>>>>> do:
>>>>>>>>>
>>>>>>>>> - It does provide a snapshot of the dependency tree that can
>>>> be
>>>>>>>>> committed into source control to avoid automatic updates of
>>>>>> (transitive)
>>>>>>>>> dependencies break the build _during development and CI_
>>>>>>>>> - It _does not_ ensure that a user installing the published
>>>>>> package gets
>>>>>>>>> exactly the dependency versions that are specified in
>>>>>> package-lock.json.
>>>>>>>>> That is what npm-shrinkwrap.json [5] is for.
>>>>>>>>> - It does speed up installation of dependencies in
>>>> conjunction
>>>>>> with `npm
>>>>>>>>> ci` by skipping the entire dependency resolution and using
>>>> the
>>>>>> versions
>>>>>>>>> from the lock file.
>>>>>>>>> - It is required to be present for `npm audit` [6], although
>>>> it
>>>>>> could be
>>>>>>>>> generated ad-hoc.
>>>>>>>>> - It is possible to manually tinker with the lock file to fix
>>>>>> audit
>>>>>>>>> issues with transitive dependencies that have no update
>>>>>> available. This
>>>>>>>>> requires some special care to prevent npm from resetting
>>>> these
>>>>>> manual
>>>>>>>>> changes, but it's a valuable last-resort option. However,
>>>> this
>>>>>> is far more
>>>>>>>>> useful with npm-shrinkwrap.json to create releases without
>>>>>> security
>>>>>>>>> issues.
>>>>>>>>>
>>>>>>>>> With that cleared up, my stance on committing package-lock.json
>>>> is
>>>>> as
>>>>>>>>> follows:
>>>>>>>>>
>>>>>>>>> - Faster CI installations and faster/easier usage of `npm
>>>>> audit`
>>>>>> are
>>>>>>>>> purely convenience features for me.
>>>>>>>>> - Consistent developer builds and updates only on-demand are
>>>>> real
>>>>>>>>> advantages for me. I just recently spent hours finding out
>>>> why
>>>>>> some tests
>>>>>>>>> of cordova-lib were suddenly failing. It turned out it was
>>>>>> caused by an
>>>>>>>>> update to `jasmine@3.2.0`.
>>>>>>>>> - If the package-lock.json really should interfere with our
>>>>>> ability to
>>>>>>>>> link repositories for development, then that would be a deal
>>>>>> breaker for me.
>>>>>>>>> However, the primary goal that I wanted to achieve was _immutable
>>>>>>>>> releases_. That is, installing e.g. `cordova@9` should _always
>>>>>> install the
>>>>>>>>> exact same set of packages_. What we need for that is
>>>>>> npm-shrinkwrap.json.
>>>>>>>>> So IMO whether we decide for or against using package-lock.json,
>>>> we
>>>>>> should
>>>>>>>>> lock down the dependencies for releases of our CLIs, platforms
>>>> and
>>>>>> possibly
>>>>>>>>> plugins by generating and committing a npm-shrinkwrap.json to the
>>>>>> _release
>>>>>>>>> branch_ before packaging the release.
>>>>>>>>>
>>>>>>>>> Cheers,
>>>>>>>>> Raphael
>>>>>>>>>
>>>>>>>>> [1]:
>>>>>> https://github.com/apache/cordova-cli/pull/325#issuecomment-421336025
>>>>>>>>> [2]:
>>>>>>>>>
>>>> https://github.com/raphinesse/cordova-common/pull/1#issuecomment-420950433
>>>>>>>>> [3]: https://docs.npmjs.com/files/package-locks
>>>>>>>>> [4]: https://docs.npmjs.com/files/package-lock.json
>>>>>>>>> [5]: https://docs.npmjs.com/files/shrinkwrap.json
>>>>>>>>> [6]:
>>>>> https://docs.npmjs.com/getting-started/running-a-security-audit
>>>>>>>>
>>>> ---------------------------------------------------------------------
>>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
>>>>>>>> For additional commands, e-mail: dev-help@cordova.apache.org
>>>>>>>>
>>>>>> ---------------------------------------------------------------------
>>>>>> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
>>>>>> For additional commands, e-mail: dev-help@cordova.apache.org
>>>>>>
>>>>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
>> For additional commands, e-mail: dev-help@cordova.apache.org
>>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> For additional commands, e-mail: dev-help@cordova.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org