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://docs.npmjs.com/cli/shrinkwrap On Wed, Oct 28, 2015 at 8:37 PM Steven Gill <stevengil...@gmail.com> wrote: > +1 nice conclusion. > > On Wed, Oct 28, 2015 at 5:30 PM, Jesse <purplecabb...@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 > > risingj.com > > > > On Wed, Oct 28, 2015 at 5:28 PM, Dmitry Blotsky <dblot...@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 <purplecabb...@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 < > > dblot...@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 <stevengil...@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 < > > > dblot...@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 < > stevengil...@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 < > > > >> dblot...@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 < > > > dblot...@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 > > > >>>>>> > > > >>>>>> > > > >>>> > > > >>>> > > > >> > > > >> > > > > > > > > >
