+1 to shrinkwrap before publish
On 2018-09-15 00:27, raphine...@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 <purplecabb...@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 <raphine...@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.br...@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 <chris.br...@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
<oliver.salzb...@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, raphine...@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-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
