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 <[email protected]> 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 <[email protected]> wrote: > >> +1 nice conclusion. >> >> On Wed, Oct 28, 2015 at 5:30 PM, Jesse <[email protected]> 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 <[email protected]> >>> 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 <[email protected]> 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 < >>> [email protected]> >>>>> 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 <[email protected]> >>>> 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 < >>>> [email protected]> >>>>>>> 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 < >> [email protected]> >>>>>>>> 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 < >>>>>> [email protected]> >>>>>>>>> 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 < >>>> [email protected]> >>>>>>>>>> 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 >>>>>>>>>> >>>>>>>>>> >>>>>>>> >>>>>>>> >>>>>> >>>>>> >>>> >>>> >>> >>
