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/08/01 20:28:38 UTC

Commit package-lock.json in next major Cordova release or not?

I think we should start to commit package-lock.json in the next major
release but am not 100% sure. My understanding is that
package-lock.json mostly serves a couple major purposes:
* preserve the structure of node_modules cross-platform
* use SHA numbers to verify correct packages

There seem to have been changes between npm@4 (??), npm@5, and npm@6,
as described in the following:
* https://github.com/npm/npm/issues/20434 (npm@5 vs npm@6)
* https://jpospisil.com/2017/06/02/understanding-lock-files-in-npm-5.html

From what I read I think npm@5 & npm@6 would continue to follow the
semver rules for packages specified in package.json.

Major advantages I can think of:
* better consistency for cross-platform development
* no need to regenerate package-lock.json for npm audit check

But I can think of the following possible disadvantages to consider:
* not as easy to update dependencies, probably not possible to just
update dependencies by hand
* some additional "noise" in the git history, shouldn't be too bad though
* possibly major: in case people work on different dependency changes
in parallel and want to merge by git merge, rebase, or cherry-pick
dealing with the package-lock.json changes may not be so clean

and a counter-point:
* https://www.codementor.io/johnkennedy/get-rid-of-that-npm-package-lock-json-e0bj7ai42

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


Re: Commit package-lock.json in next major Cordova release or not?

Posted by Oliver Salzburg <ol...@gmail.com>.
I would agree with your conclusion. Except that my personal 
interpretation of it was that npm is as stable as a hovercraft filled 
with eels. :D

On 2018-09-17 13:22, Chris Brody wrote:
> On Mon, Sep 17, 2018 at 5:35 AM Oliver Salzburg
> <ol...@gmail.com> wrote:
>> I would also suggest to have a brief look at the issues other users are
>> reporting regarding this:
>>
>> - https://github.com/npm/npm/issues?utf8=%E2%9C%93&q=is%3Aissue+package-lock
>> - https://npm.community/search?q=package-lock%20category%3A6
> I see that there has been quite a bit of confusion around the package
> lock behavior, and maybe some bugs found, in the past. The package
> lock behavior seems to have changed over the past few npm releases,
> which indicates to me that they would have fixed some bugs as well. Is
> there anything else we can conclude from these links?
>
>> Of course, in the end, nothing beats a validation against own targets.
> Agreed on my part.
>
> ---------------------------------------------------------------------
> 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: Commit package-lock.json in next major Cordova release or not?

Posted by Chris Brody <ch...@gmail.com>.
On Mon, Sep 17, 2018 at 5:35 AM Oliver Salzburg
<ol...@gmail.com> wrote:
>
> I would also suggest to have a brief look at the issues other users are
> reporting regarding this:
>
> - https://github.com/npm/npm/issues?utf8=%E2%9C%93&q=is%3Aissue+package-lock
> - https://npm.community/search?q=package-lock%20category%3A6

I see that there has been quite a bit of confusion around the package
lock behavior, and maybe some bugs found, in the past. The package
lock behavior seems to have changed over the past few npm releases,
which indicates to me that they would have fixed some bugs as well. Is
there anything else we can conclude from these links?

> Of course, in the end, nothing beats a validation against own targets.

Agreed on my part.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


Re: Commit package-lock.json in next major Cordova release or not?

Posted by Oliver Salzburg <ol...@gmail.com>.
I would also suggest to have a brief look at the issues other users are 
reporting regarding this:

- https://github.com/npm/npm/issues?utf8=%E2%9C%93&q=is%3Aissue+package-lock
- https://npm.community/search?q=package-lock%20category%3A6

Of course, in the end, nothing beats a validation against own targets. 
I'm probably the wrong guy for that job though :D

On 2018-09-14 21:53, Chris Brody 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


Re: Commit package-lock.json in next major Cordova release or not?

Posted by Oliver Salzburg <ol...@gmail.com>.
+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


Re: Commit package-lock.json in next major Cordova release or not?

Posted by Chris Brody <ch...@gmail.com>.
> Please double check the docs for the use cases of
> package-lock.json vs npm-shrinkwrap.json.

Adding links to some resources I found, for the sake of clarity:

* <https://github.com/npm/cli/blob/latest/doc/spec/package-lock.md>
* <https://stackoverflow.com/questions/44258235/what-is-the-difference-between-npm-shrinkwrap-json-and-package-lock-json/46132512#46132512>
(I just made an edit to update a link, now waiting for peer review)
* <https://docs.npmjs.com/files/package-locks>
* <https://docs.npmjs.com/files/package-lock.json> (with link to a
long "so you want to write a package manager" article which looks
worth reading)

This could probably be a bit better organized, hope I get a chance to
take a better look (someday).

On Fri, Sep 14, 2018 at 6:39 PM Jesse <pu...@gmail.com> wrote:
>
> +1
> Thank you for the clarity! I am completely supportive of this.
>
> @purplecabbage
> risingj.com
>
>
> On Fri, Sep 14, 2018 at 3:28 PM <ra...@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


Re: Commit package-lock.json in next major Cordova release or not?

Posted by Jesse <pu...@gmail.com>.
+1
Thank you for the clarity! I am completely supportive of this.

@purplecabbage
risingj.com


On Fri, Sep 14, 2018 at 3:28 PM <ra...@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
> > > >
> > > >
> > >
> >
>

Re: Commit package-lock.json in next major Cordova release or not?

Posted by ra...@gmail.com.
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
> > >
> > >
> >
>

Re: Commit package-lock.json in next major Cordova release or not?

Posted by 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
> >
> >
>

Re: Commit package-lock.json in next major Cordova release or not?

Posted by ra...@gmail.com.
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
>
>

Re: Commit package-lock.json in next major Cordova release or not?

Posted by Chris Brody <ch...@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


Re: Commit package-lock.json in next major Cordova release or not?

Posted by Chris Brody <ch...@gmail.com>.
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


Re: Commit package-lock.json in next major Cordova release or not?

Posted by Oliver Salzburg <ol...@gmail.com>.
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


Re: Commit package-lock.json in next major Cordova release or not?

Posted by ra...@gmail.com.
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

Re: Commit package-lock.json in next major Cordova release or not?

Posted by Chris Brody <ch...@gmail.com>.
I would like to voice a couple more concerns about this idea, despite
the fact that we had already reached agreement and I have started
participating in this task [1]
(<https://github.com/apache/cordova/issues/4>). My apologies for such
bad timing.

My major concern is that npm does not seem to be so smart about
updating intermediate dependencies in package-lock.json. I made an
illustration in [2]
(<https://github.com/apache/cordova-cli/pull/325>).

While npm audit does give us instructions how to update intermediate
dependencies I find this to be a bit clumsy and non-intuitive.

Assuming we do move forward and commit package-lock.json in the
Cordova repositories, a possible solution would be to periodically
delete and regenerate package-lock.json ([3]).

I would also like to quote a concern pasted below, from another thread
[4], which I think does have a minor level of merit.

[1] https://github.com/apache/cordova/issues/4
[2] https://github.com/apache/cordova-cli/pull/325
[3] https://github.com/apache/cordova-cli/pull/325#issuecomment-421089293
[4] https://lists.apache.org/thread.html/ef3872ea5d1147df21ab010ab3c579a655e86af18fcd3e93598a8837@%3Cdev.cordova.apache.org%3E

On Tue, Sep 11, 2018 at 7:14 AM Oliver Salzburg <***> wrote:
>
> I just wanted to voice my concern regarding package-locks on the list,
> even though consensus was probably reached in the past already.
>
>  From our experience, this is almost as bad as committing node_modules
> into VCS.
> I understand the idea behind them and I would agree with that idea, but
> the implementation is horrible and suffers from many defects that
> ultimately lead to developer frustration and hard-to-analyze bugs.
>
> Especially during development where you might switch between package
> versions or link locally against development checkouts of modules,
> package-lock files constantly get corrupted or operations even lead to
> local development checkouts being replaced by cached npm modules,
> because the lockfile had them marked as bundled.
> And anytime something like that happens, you're left with having to
> rebuild the lockfile from scratch, introducing exactly the changes you
> didn't want in the first place.
>
> I understand that `npm ci` can have performance benefits during
> installation of packages, but the underlying technologies are defective,
> the time you save in CI will be spent by developers instead and you'll
> likely roll back or have to deal with blocking npm issues for some time.
>
> That was the experience for us and I'd hate to see others make the same
> mistake. If you go with it, I hope it works better for you and maybe I
> will learn how it was all our own fault all along and we've just been
> holding our iPhone the wrong way. :D
>
> Cheers

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


Re: Commit package-lock.json in next major Cordova release or not?

Posted by Chris Brody <ch...@gmail.com>.
Now tracked in: https://github.com/apache/cordova/issues/4

On Thu, Aug 2, 2018 at 6:14 AM Jan Piotrowski <pi...@gmail.com> wrote:

> +1 - everything that modernizes the tooling that can be used and
> actually uses its added functionality is a win.
>
> 2018-08-02 7:43 GMT+02:00 Chris Brody <ch...@gmail.com>:
> > Just raised https://issues.apache.org/jira/browse/CB-14248
> >
> > Thanks for the helpful responses
> > On Thu, Aug 2, 2018 at 12:12 AM Shazron <sh...@gmail.com> wrote:
> >>
> >> yes +1
> >> On Thu, Aug 2, 2018 at 4:28 AM Chris Brody <ch...@gmail.com>
> wrote:
> >> >
> >> > I think we should start to commit package-lock.json in the next major
> >> > release but am not 100% sure. My understanding is that
> >> > package-lock.json mostly serves a couple major purposes:
> >> > * preserve the structure of node_modules cross-platform
> >> > * use SHA numbers to verify correct packages
> >> >
> >> > There seem to have been changes between npm@4 (??), npm@5, and npm@6,
> >> > as described in the following:
> >> > * https://github.com/npm/npm/issues/20434 (npm@5 vs npm@6)
> >> > *
> https://jpospisil.com/2017/06/02/understanding-lock-files-in-npm-5.html
> >> >
> >> > From what I read I think npm@5 & npm@6 would continue to follow the
> >> > semver rules for packages specified in package.json.
> >> >
> >> > Major advantages I can think of:
> >> > * better consistency for cross-platform development
> >> > * no need to regenerate package-lock.json for npm audit check
> >> >
> >> > But I can think of the following possible disadvantages to consider:
> >> > * not as easy to update dependencies, probably not possible to just
> >> > update dependencies by hand
> >> > * some additional "noise" in the git history, shouldn't be too bad
> though
> >> > * possibly major: in case people work on different dependency changes
> >> > in parallel and want to merge by git merge, rebase, or cherry-pick
> >> > dealing with the package-lock.json changes may not be so clean
> >> >
> >> > and a counter-point:
> >> > *
> https://www.codementor.io/johnkennedy/get-rid-of-that-npm-package-lock-json-e0bj7ai42
> >> >
> >> > ---------------------------------------------------------------------
> >> > 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: Commit package-lock.json in next major Cordova release or not?

Posted by Jan Piotrowski <pi...@gmail.com>.
+1 - everything that modernizes the tooling that can be used and
actually uses its added functionality is a win.

2018-08-02 7:43 GMT+02:00 Chris Brody <ch...@gmail.com>:
> Just raised https://issues.apache.org/jira/browse/CB-14248
>
> Thanks for the helpful responses
> On Thu, Aug 2, 2018 at 12:12 AM Shazron <sh...@gmail.com> wrote:
>>
>> yes +1
>> On Thu, Aug 2, 2018 at 4:28 AM Chris Brody <ch...@gmail.com> wrote:
>> >
>> > I think we should start to commit package-lock.json in the next major
>> > release but am not 100% sure. My understanding is that
>> > package-lock.json mostly serves a couple major purposes:
>> > * preserve the structure of node_modules cross-platform
>> > * use SHA numbers to verify correct packages
>> >
>> > There seem to have been changes between npm@4 (??), npm@5, and npm@6,
>> > as described in the following:
>> > * https://github.com/npm/npm/issues/20434 (npm@5 vs npm@6)
>> > * https://jpospisil.com/2017/06/02/understanding-lock-files-in-npm-5.html
>> >
>> > From what I read I think npm@5 & npm@6 would continue to follow the
>> > semver rules for packages specified in package.json.
>> >
>> > Major advantages I can think of:
>> > * better consistency for cross-platform development
>> > * no need to regenerate package-lock.json for npm audit check
>> >
>> > But I can think of the following possible disadvantages to consider:
>> > * not as easy to update dependencies, probably not possible to just
>> > update dependencies by hand
>> > * some additional "noise" in the git history, shouldn't be too bad though
>> > * possibly major: in case people work on different dependency changes
>> > in parallel and want to merge by git merge, rebase, or cherry-pick
>> > dealing with the package-lock.json changes may not be so clean
>> >
>> > and a counter-point:
>> > * https://www.codementor.io/johnkennedy/get-rid-of-that-npm-package-lock-json-e0bj7ai42
>> >
>> > ---------------------------------------------------------------------
>> > 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: Commit package-lock.json in next major Cordova release or not?

Posted by Chris Brody <ch...@gmail.com>.
Just raised https://issues.apache.org/jira/browse/CB-14248

Thanks for the helpful responses
On Thu, Aug 2, 2018 at 12:12 AM Shazron <sh...@gmail.com> wrote:
>
> yes +1
> On Thu, Aug 2, 2018 at 4:28 AM Chris Brody <ch...@gmail.com> wrote:
> >
> > I think we should start to commit package-lock.json in the next major
> > release but am not 100% sure. My understanding is that
> > package-lock.json mostly serves a couple major purposes:
> > * preserve the structure of node_modules cross-platform
> > * use SHA numbers to verify correct packages
> >
> > There seem to have been changes between npm@4 (??), npm@5, and npm@6,
> > as described in the following:
> > * https://github.com/npm/npm/issues/20434 (npm@5 vs npm@6)
> > * https://jpospisil.com/2017/06/02/understanding-lock-files-in-npm-5.html
> >
> > From what I read I think npm@5 & npm@6 would continue to follow the
> > semver rules for packages specified in package.json.
> >
> > Major advantages I can think of:
> > * better consistency for cross-platform development
> > * no need to regenerate package-lock.json for npm audit check
> >
> > But I can think of the following possible disadvantages to consider:
> > * not as easy to update dependencies, probably not possible to just
> > update dependencies by hand
> > * some additional "noise" in the git history, shouldn't be too bad though
> > * possibly major: in case people work on different dependency changes
> > in parallel and want to merge by git merge, rebase, or cherry-pick
> > dealing with the package-lock.json changes may not be so clean
> >
> > and a counter-point:
> > * https://www.codementor.io/johnkennedy/get-rid-of-that-npm-package-lock-json-e0bj7ai42
> >
> > ---------------------------------------------------------------------
> > 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: Commit package-lock.json in next major Cordova release or not?

Posted by Shazron <sh...@gmail.com>.
yes +1
On Thu, Aug 2, 2018 at 4:28 AM Chris Brody <ch...@gmail.com> wrote:
>
> I think we should start to commit package-lock.json in the next major
> release but am not 100% sure. My understanding is that
> package-lock.json mostly serves a couple major purposes:
> * preserve the structure of node_modules cross-platform
> * use SHA numbers to verify correct packages
>
> There seem to have been changes between npm@4 (??), npm@5, and npm@6,
> as described in the following:
> * https://github.com/npm/npm/issues/20434 (npm@5 vs npm@6)
> * https://jpospisil.com/2017/06/02/understanding-lock-files-in-npm-5.html
>
> From what I read I think npm@5 & npm@6 would continue to follow the
> semver rules for packages specified in package.json.
>
> Major advantages I can think of:
> * better consistency for cross-platform development
> * no need to regenerate package-lock.json for npm audit check
>
> But I can think of the following possible disadvantages to consider:
> * not as easy to update dependencies, probably not possible to just
> update dependencies by hand
> * some additional "noise" in the git history, shouldn't be too bad though
> * possibly major: in case people work on different dependency changes
> in parallel and want to merge by git merge, rebase, or cherry-pick
> dealing with the package-lock.json changes may not be so clean
>
> and a counter-point:
> * https://www.codementor.io/johnkennedy/get-rid-of-that-npm-package-lock-json-e0bj7ai42
>
> ---------------------------------------------------------------------
> 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: Commit package-lock.json in next major Cordova release or not?

Posted by Bryan Ellis <el...@gmail.com>.
I also agree that the package-lock.json should be committed.

There are a few NPM commands that were released back in NPM 5.7.0+ that I feel would be worth using in our CI testing. These commands require either a package-lock.json or shrink-wrap. 

Commands:
npm ci
npm cit => npm ci && npm t

These commands are for continuous integration testing. 
https://docs.npmjs.com/cli/ci

The downside is that the NPM version released with the latest Node 8 is 5.6.0 and does not have these commands.

If we want to try and start using these commands, I recommend only on our Node 10 runs. I believe we should not upgrade the NPM version that comes with the latest install of Node 6 and 8. Users may not upgrade NPM. The best test case I feel would be to use what is bundled with the Node installation.

- Bryan

> On Aug 2, 2018, at 7:05, raphinesse@gmail.com wrote:
> 
> Yes, we should definitely commit package-lock.json. Gives reliable builds
> and immutable releases.
> 
> Conflicts during merges are common and a bit annoying, but easily resolved
> if you know how. There even is a git merge hook for that [1] (more info on
> this blog post [2])
> 
> [1]: https://www.npmjs.com/package/npm-merge-driver
> [2]:
> https://blog.npmjs.org/post/173240511455/the-new-npm-cli-a-year-in-review-or-what-you
> 
> Jesse <pu...@gmail.com> schrieb am Mi., 1. Aug. 2018, 23:50:
> 
>> Yes, commit package-lock.json is the recommended practice, especially since
>> we are not bundling.
>> This ensures we all have the exact same version of all dependencies, which
>> is the major benefit in my mind.
>> If we just depend on package.json, some of the deeper dependency versions
>> with wild card versions can differ based on when the developer ran npm i.
>> 
>> The only disadvantage I see is:
>> - I have gotten used to using autocomplete when I type `cat pac` to see the
>> contents of package.json, now I have to type a little more.
>> - If anyone uses yarn, this file is meaningless to them, and they might
>> want us to commit yarn.lock also.
>> 
>> 
>> 
>> 
>> 
>> @purplecabbage
>> risingj.com
>> 
>> On Wed, Aug 1, 2018 at 1:28 PM, Chris Brody <ch...@gmail.com> wrote:
>> 
>>> I think we should start to commit package-lock.json in the next major
>>> release but am not 100% sure. My understanding is that
>>> package-lock.json mostly serves a couple major purposes:
>>> * preserve the structure of node_modules cross-platform
>>> * use SHA numbers to verify correct packages
>>> 
>>> There seem to have been changes between npm@4 (??), npm@5, and npm@6,
>>> as described in the following:
>>> * https://github.com/npm/npm/issues/20434 (npm@5 vs npm@6)
>>> *
>> https://jpospisil.com/2017/06/02/understanding-lock-files-in-npm-5.html
>>> 
>>> From what I read I think npm@5 & npm@6 would continue to follow the
>>> semver rules for packages specified in package.json.
>>> 
>>> Major advantages I can think of:
>>> * better consistency for cross-platform development
>>> * no need to regenerate package-lock.json for npm audit check
>>> 
>>> But I can think of the following possible disadvantages to consider:
>>> * not as easy to update dependencies, probably not possible to just
>>> update dependencies by hand
>>> * some additional "noise" in the git history, shouldn't be too bad though
>>> * possibly major: in case people work on different dependency changes
>>> in parallel and want to merge by git merge, rebase, or cherry-pick
>>> dealing with the package-lock.json changes may not be so clean
>>> 
>>> and a counter-point:
>>> * https://www.codementor.io/johnkennedy/get-rid-of-that-
>>> npm-package-lock-json-e0bj7ai42
>>> 
>>> ---------------------------------------------------------------------
>>> 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: Commit package-lock.json in next major Cordova release or not?

Posted by ra...@gmail.com.
Yes, we should definitely commit package-lock.json. Gives reliable builds
and immutable releases.

Conflicts during merges are common and a bit annoying, but easily resolved
if you know how. There even is a git merge hook for that [1] (more info on
this blog post [2])

[1]: https://www.npmjs.com/package/npm-merge-driver
[2]:
https://blog.npmjs.org/post/173240511455/the-new-npm-cli-a-year-in-review-or-what-you

Jesse <pu...@gmail.com> schrieb am Mi., 1. Aug. 2018, 23:50:

> Yes, commit package-lock.json is the recommended practice, especially since
> we are not bundling.
> This ensures we all have the exact same version of all dependencies, which
> is the major benefit in my mind.
> If we just depend on package.json, some of the deeper dependency versions
> with wild card versions can differ based on when the developer ran npm i.
>
> The only disadvantage I see is:
> - I have gotten used to using autocomplete when I type `cat pac` to see the
> contents of package.json, now I have to type a little more.
> - If anyone uses yarn, this file is meaningless to them, and they might
> want us to commit yarn.lock also.
>
>
>
>
>
> @purplecabbage
> risingj.com
>
> On Wed, Aug 1, 2018 at 1:28 PM, Chris Brody <ch...@gmail.com> wrote:
>
> > I think we should start to commit package-lock.json in the next major
> > release but am not 100% sure. My understanding is that
> > package-lock.json mostly serves a couple major purposes:
> > * preserve the structure of node_modules cross-platform
> > * use SHA numbers to verify correct packages
> >
> > There seem to have been changes between npm@4 (??), npm@5, and npm@6,
> > as described in the following:
> > * https://github.com/npm/npm/issues/20434 (npm@5 vs npm@6)
> > *
> https://jpospisil.com/2017/06/02/understanding-lock-files-in-npm-5.html
> >
> > From what I read I think npm@5 & npm@6 would continue to follow the
> > semver rules for packages specified in package.json.
> >
> > Major advantages I can think of:
> > * better consistency for cross-platform development
> > * no need to regenerate package-lock.json for npm audit check
> >
> > But I can think of the following possible disadvantages to consider:
> > * not as easy to update dependencies, probably not possible to just
> > update dependencies by hand
> > * some additional "noise" in the git history, shouldn't be too bad though
> > * possibly major: in case people work on different dependency changes
> > in parallel and want to merge by git merge, rebase, or cherry-pick
> > dealing with the package-lock.json changes may not be so clean
> >
> > and a counter-point:
> > * https://www.codementor.io/johnkennedy/get-rid-of-that-
> > npm-package-lock-json-e0bj7ai42
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> > For additional commands, e-mail: dev-help@cordova.apache.org
> >
> >
>

Re: Commit package-lock.json in next major Cordova release or not?

Posted by Jesse <pu...@gmail.com>.
Yes, commit package-lock.json is the recommended practice, especially since
we are not bundling.
This ensures we all have the exact same version of all dependencies, which
is the major benefit in my mind.
If we just depend on package.json, some of the deeper dependency versions
with wild card versions can differ based on when the developer ran npm i.

The only disadvantage I see is:
- I have gotten used to using autocomplete when I type `cat pac` to see the
contents of package.json, now I have to type a little more.
- If anyone uses yarn, this file is meaningless to them, and they might
want us to commit yarn.lock also.





@purplecabbage
risingj.com

On Wed, Aug 1, 2018 at 1:28 PM, Chris Brody <ch...@gmail.com> wrote:

> I think we should start to commit package-lock.json in the next major
> release but am not 100% sure. My understanding is that
> package-lock.json mostly serves a couple major purposes:
> * preserve the structure of node_modules cross-platform
> * use SHA numbers to verify correct packages
>
> There seem to have been changes between npm@4 (??), npm@5, and npm@6,
> as described in the following:
> * https://github.com/npm/npm/issues/20434 (npm@5 vs npm@6)
> * https://jpospisil.com/2017/06/02/understanding-lock-files-in-npm-5.html
>
> From what I read I think npm@5 & npm@6 would continue to follow the
> semver rules for packages specified in package.json.
>
> Major advantages I can think of:
> * better consistency for cross-platform development
> * no need to regenerate package-lock.json for npm audit check
>
> But I can think of the following possible disadvantages to consider:
> * not as easy to update dependencies, probably not possible to just
> update dependencies by hand
> * some additional "noise" in the git history, shouldn't be too bad though
> * possibly major: in case people work on different dependency changes
> in parallel and want to merge by git merge, rebase, or cherry-pick
> dealing with the package-lock.json changes may not be so clean
>
> and a counter-point:
> * https://www.codementor.io/johnkennedy/get-rid-of-that-
> npm-package-lock-json-e0bj7ai42
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> For additional commands, e-mail: dev-help@cordova.apache.org
>
>