Additionally, I think it will only be a problem when each of the modules call into one another. I.e. when cordova-cli calls into cordova-plugman and that throws a CordovaError which is caught by cordova-cli and then compared with instanceof to its own version.. Its not a problem in isolation.
This problem usually comes up with singletons / internal caching, such as reading config files from disk once and then modifying in-memory, but I'm not sure if we actyally have that problem. -Michal On Fri, Jun 6, 2014 at 1:12 PM, Andrew Grieve <[email protected]> wrote: > On Fri, Jun 6, 2014 at 12:37 PM, Brian LeRoux <[email protected]> wrote: > > > Ya I spoke to Issac a bit about this last week and he wasn't being > entirely > > facetious. One path that apparently works well is to treat root > > `./node_modules` as something npm manages and then nested node_modules > for > > userspace stuff. > > > > eg. `./src/node_modules` and then `var foo = require('src/foo')` > > > > Not a huge win over just using relative paths so meh. > > > > I don't understand why instanceof wouldn't work? > > > > You probably have a better understanding of this than I do... But: > - There's no problem when running locally, since we've defined the > structure of node_modules to work, > > But when npm install is run on an end-users machine, if it ends up like: > > cordova-lib/node_modules/cordova-plugman/node_modules/CordovaError > cordova-lib/node_modules/cordova-cli/node_modules/CordovaError > cordova-lib/node_modules/CordovaError > > then there are now 3 CordovaError constructors, so instanceof will not work > between them. > > Now, if npm sees that all packages call for the same version of > CordovaError, and install them like: > > cordova-lib/node_modules/cordova-plugman > cordova-lib/node_modules/cordova-cli > cordova-lib/node_modules/CordovaError > > then all is well. > > Easy to answer with a quick test :) > > > > > > > > > On Fri, Jun 6, 2014 at 9:14 AM, Andrew Grieve <[email protected]> > > wrote: > > > > > Exciting! I tried the "check in node_modules" approach in > > > cordova-app-harness and it works really well there: > > > https://github.com/apache/cordova-app-harness/tree/master/harness-push > > > It's a simpler case since there's a strict parent/child relationship. > > > > > > The main unknown for me is how to make CordovaError work as its own > > module > > > since we'll require that there is never multiple copies of the module > > when > > > the end-user types "npm install" (or else the instanceof check won't > > work). > > > I *think* that npm does the right thing so long as all of the packages > > use > > > the same version constraints for it, but I haven't tested it. > > > > > > > > > On Fri, Jun 6, 2014 at 11:37 AM, Michal Mocny <[email protected]> > > wrote: > > > > > > > Brian: sounds good. Looking forward to see where we get. > > > > > > > > -Michal > > > > > > > > > > > > On Fri, Jun 6, 2014 at 11:36 AM, Brian LeRoux <[email protected]> wrote: > > > > > > > > > @Michal: we're going to start extracting things to their own > modules. > > > (As > > > > > discussed.) We're going to start small and simple: CordovaError, > > > > > SuperSpawn, etc. But before we do that we'll try to merge up as > many > > > PRs > > > > as > > > > > possible. Ideally there are none when we get rolling. (Next week.) > > > > > > > > > > Many repos (or just small modules) would isolate the changes making > > it > > > > > easier to refactor. Anyhow: I'm tired of advocating that design > > pattern > > > > > choice and I'm sure you're tired of hearing about it! > > > > > > > > > > > > > > > On Thu, Jun 5, 2014 at 6:04 PM, Michal Mocny <[email protected]> > > > > wrote: > > > > > > > > > > > On Thu, Jun 5, 2014 at 6:47 PM, Brian LeRoux <[email protected]> wrote: > > > > > > > > > > > > > we're about to do some heavy refactoring on cordova-lib…it > would > > > > appear > > > > > > > many ppl are working on it atm: > > > > > > > > > > > > > > > > > > > Who/how/what/why? > > > > > > > > > > > > > > > > > > > > > > > > > > https://github.com/apache/cordova-lib/pulls > > > > > > > > > > > > > > I'm afraid to find out how many *aren't* open PR's …but this > > > > certainly > > > > > > > illustrates the problem with one giant repo with all our code > > > > > > > > > > > > > > > > > > > If you are refactoring the code in a way that breaks PR from > > > applying, > > > > I > > > > > > don't think multiple repos would make a difference. Probably > this > > > way > > > > is > > > > > > easier if you were making changes that needed to touch multiple > > > repos. > > > > > But > > > > > > this conversation feels sorta Deja Vu. > > > > > > > > > > > > -Michal > > > > > > > > > > > > > > > > > > > > >
