cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dmitry Blotsky <dblot...@microsoft.com>
Subject Re: [DISCUSS] Copying node_modules during platform install
Date Thu, 29 Oct 2015 19:48:01 GMT
Gotcha. I’m now thoroughly convinced that shipping node_modules directly is the 100% way
to go. I guess having out of date dependencies is just an unavoidable side effect of shipping
software as-is. We’ll just have to be diligent in updating cordova-common in platforms quickly
enough to unblock CI when bugs come up. Thanks for the detailed explanation, Carlos.

Kindly,
Dmitry

> On Oct 28, 2015, at 8:08 PM, Carlos Santana <csantana23@gmail.com> wrote:
> 
> Sorry for being late to the party.
> +1 to conclusion
> 
> But leaving 2 cents
> 
> - Using "npm install" or "npm link" is a development time (i.e. cordova
> contributors) only, When a product is shipped we should not rely on using
> this process to satisfy dependencies
> - The way dependencies are manage in npm/node is not controllable by
> package.json, most people they think they can by fixing/pinning the
> dependencies versions, but what they don't realize is that the dependencies
> children/graph don't usually use fixed versions of their dependencies and
> so on thru the whole deep graph
> - This is why npm-shrinkwrap exist [1] in the first place
> - Bundling all dependencies there are multiple benefits
> 1. we shipped the exact version of the dependency that we clear with legal
> 2. we avoid less headaches for end user to be responsible to install
> dependencies on the fly
> 3. we shipped what we tested
> 4. every end user get's the exact version of the software, an user running
> cordova 5.3.3 that install a week ago, using same bytes of software of a
> user that installed cordova 5.3.3 yesterday
> Making 3 and 4, the most important to me.
> 
> [1] https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fdocs.npmjs.com%2fcli%2fshrinkwrap&data=01%7c01%7cdblotsky%40microsoft.com%7cc13f7ce7572044ed272a08d2e00e3856%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=1CsQ118SBKKLj0YcYUV9TMt0Gjlv1UUBfwYHj4tmUmo%3d
> 
> 
> 
> On Wed, Oct 28, 2015 at 8:37 PM Steven Gill <stevengill97@gmail.com> wrote:
> 
>> +1 nice conclusion.
>> 
>> On Wed, Oct 28, 2015 at 5:30 PM, Jesse <purplecabbage@gmail.com> wrote:
>> 
>>> Yes, I believe the cordova-lib case should be linked, and definitely this
>>> is where a relative link makes sense, since it's all in the same repo.
>>> 
>>> 
>>> @purplecabbage
>>> https://na01.safelinks.protection.outlook.com/?url=risingj.com&data=01%7c01%7cdblotsky%40microsoft.com%7cc13f7ce7572044ed272a08d2e00e3856%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=8JUuLcFvlvO%2b5n9S78bS7yO0UZdl%2fzhkMS04lay%2fPyo%3d
>>> 
>>> On Wed, Oct 28, 2015 at 5:28 PM, Dmitry Blotsky <dblotsky@microsoft.com>
>>> wrote:
>>> 
>>>>> trust that each platform will be updated
>>>>> with a newer cordova-common when it makes sense
>>>> 
>>>> That sounds reasonable to me. I’m removing the link step for
>>>> cordova-common in platforms.
>>>> 
>>>> Last question: the cordova-common used by cordova-lib should still be
>>>> linked, yes?
>>>> 
>>>> Kindly,
>>>> Dmitry
>>>> 
>>>>> On Oct 28, 2015, at 2:44 PM, Jesse <purplecabbage@gmail.com> wrote:
>>>>> 
>>>>> It is completely likely, and maybe even expected that the specific
>>>> version
>>>>> of cordova-common that sits in any particular platform template can
>> be
>>>>> different than what is used by cordova-lib.
>>>>> They may even be different amongst platforms in a single project.
>>>>> 
>>>>> As long as the lib->platform api is consistent, this is not an issue.
>>> We
>>>>> use cordova-common solely to reduce duplicated code in our repos; We
>> do
>>>> not
>>>>> use cordova-common to reduce the duplicated code on app developers'
>>>>> machines between multiple projects.
>>>>> 
>>>>> re: >> Follow-up: the reason we link them is so that we test master
>>> with
>>>>> master at all times.
>>>>> I would change this so that a platform is tested as a complete
>> entity.
>>>> Test
>>>>> what is in platform-master and trust that each platform will be
>> updated
>>>>> with a newer cordova-common when it makes sense.
>>>>> 
>>>>> 
>>>>> 
>>>>> @purplecabbage
>>>>> 
>>>> 
>>> 
>> https://na01.safelinks.protection.outlook.com/?url=risingj.com&data=01%7c01%7cdblotsky%40microsoft.com%7c25dac595182e43deb77b08d2dfe0ee9c%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=9qhpB1FZ258BqXXE0vEepVP%2fR%2bbxgEViwRhTFEDm6sE%3d
>>>>> 
>>>>> On Wed, Oct 28, 2015 at 1:42 PM, Dmitry Blotsky <
>>> dblotsky@microsoft.com>
>>>>> wrote:
>>>>> 
>>>>>> This use case does mostly affect Cordova contributors. Usually the
>>>>>> platforms will come packaged, you’re right. The discussion of
>>>>>> packaged-vs-installed is separate, but just as a nod to it: I don’t
>>> see
>>>> a
>>>>>> reason we can’t just automatically call “npm install” in
>>>>>> platforms/ios/cordova. Copying over a package.json with fixed
>> versions
>>>> is
>>>>>> no more complex than copying over node_modules.
>>>>>> 
>>>>>> Kindly,
>>>>>> Dmitry
>>>>>> 
>>>>>>> On Oct 28, 2015, at 1:33 PM, Steven Gill <stevengill97@gmail.com>
>>>> wrote:
>>>>>>> 
>>>>>>> Currently, those modules ship with the platform. Example,
>> cordova-ios
>>>>>> will
>>>>>>> have all necessary modules bundled in. When you create a cordova
>>>> project
>>>>>>> and add a platform, it takes those modules and moves them into
your
>>>> newly
>>>>>>> created cordova project. To have the cordova project run `npm
>>> install`
>>>>>> and
>>>>>>> install those modules would require us include those modules
in a
>>>> project
>>>>>>> level `package.json` file for every cordova project that adds
that
>>>>>>> platform. This would confuse developers for sure. I don't think
we
>>>> would
>>>>>>> have to modify requires to have this work.
>>>>>>> 
>>>>>>> To run `npm install` on those directories would also be very
poor
>>>>>> practice.
>>>>>>> By those directories, I assume you mean
>>>>>>> `MYCORDOVAPROJECT/platforms/ios/cordova` (this is where necessary
>>>>>>> node_modules from cordova-ios get copied). It would mean we would
>>> have
>>>> to
>>>>>>> have a `package.json` be copied from `cordova-ios` into
>>>>>>> `MyCordovaProject/platforms/ios/cordova`. A cordova project would
>>> then
>>>> at
>>>>>>> least have as many package.json files as platforms have been
>> added. I
>>>>>> think
>>>>>>> this is poor practice.
>>>>>>> 
>>>>>>> Your usecase of npm linking modules in platforms
>>>>>> (cordova-ios/node_modules)
>>>>>>> that then get copied into `MYCORDOVAPROJECT/platforms/ios/cordova`
>> is
>>>>>>> unique. I still think the best solution is to add some sort of
>> check
>>> to
>>>>>>> make sure you haven't npm linked or handle the npm linked case
when
>>>>>>> copying. Guess this problem will arise more often with
>>> cordova-common.
>>>>>> That
>>>>>>> usecase pretty much only affects cordova contributors.
>>>>>>> 
>>>>>>> -Steve
>>>>>>> 
>>>>>>> On Wed, Oct 28, 2015 at 1:18 PM, Dmitry Blotsky <
>>>> dblotsky@microsoft.com>
>>>>>>> wrote:
>>>>>>> 
>>>>>>>> Is it possible to do “npm install” in those directories
instead?
>> Or
>>> to
>>>>>>>> adjust the path so that require() works with the original
>>> node_modules
>>>>>>>> directory?
>>>>>>>> 
>>>>>>>> Kindly,
>>>>>>>> Dmitry
>>>>>>>> 
>>>>>>>>> On Oct 27, 2015, at 10:31 PM, Steven Gill <
>> stevengill97@gmail.com>
>>>>>>>> wrote:
>>>>>>>>> 
>>>>>>>>> I don't think we thought of symlinks in this usecase.
Probably
>>> worth
>>>>>>>> adding
>>>>>>>>> in code that checks for symlinks before the copy. I don't
see us
>>>>>> removing
>>>>>>>>> this copy as the cordova scripts (build, run, install,
etc)
>> require
>>>>>> those
>>>>>>>>> modules.
>>>>>>>>> 
>>>>>>>>> On Tue, Oct 27, 2015 at 9:24 PM, Dmitry Blotsky <
>>>>>> dblotsky@microsoft.com>
>>>>>>>>> wrote:
>>>>>>>>> 
>>>>>>>>>> Ping. Anyone have any information on this? Is it
safe to "cp
>> -r” a
>>>>>>>>>> node_modules directory?
>>>>>>>>>> 
>>>>>>>>>> Kindly,
>>>>>>>>>> Dmitry
>>>>>>>>>> 
>>>>>>>>>>> On Oct 26, 2015, at 3:06 PM, Dmitry Blotsky <
>>>> dblotsky@microsoft.com>
>>>>>>>>>> wrote:
>>>>>>>>>>> 
>>>>>>>>>>> Hey folks,
>>>>>>>>>>> 
>>>>>>>>>>> I’ve come across a bug with symlinks and platform
installation
>>>>>>>> recently.
>>>>>>>>>> The point of interest is this line:
>>>>>>>>>> 
>>>>>>>> 
>>>>>> 
>>>> 
>>> 
>> https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgithub.com%2fapache%2fcordova-ios%2fblob%2f4039aeb6f87c6803df5814b8cdefb8c2058504a0%2fbin%2flib%2fcreate.js%23L93.&data=01%7c01%7cdblotsky%40microsoft.com%7c2b31253a23cc4d165ed808d2de51c9ef%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=cyXGPBQPr9v46gc6DMDeC9zZkzda8bSCIoinEyAmKrs%3d
>>>>>>>>>>> 
>>>>>>>>>>> Is copying node_modules a safe operation? In
my case there was
>> a
>>>>>>>>>> relative symlink inside it when it was copied and
as a result
>> some
>>>>>>>>>> dependencies broke. The symlink was created by a
previous
>>> invocation
>>>>>> of
>>>>>>>>>> “npm link”.
>>>>>>>>>>> 
>>>>>>>>>>> Kindly,
>>>>>>>>>>> Dmitry
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>> 
>>>>>>>> 
>>>>>> 
>>>>>> 
>>>> 
>>>> 
>>> 
>> 

Mime
View raw message