cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Grieve <agri...@chromium.org>
Subject Re: wip on cordova-lib? pls let us know
Date Fri, 06 Jun 2014 17:12:53 GMT
On Fri, Jun 6, 2014 at 12:37 PM, Brian LeRoux <b@brian.io> 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 <agrieve@chromium.org>
> 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 <mmocny@chromium.org>
> wrote:
> >
> > > Brian: sounds good.  Looking forward to see where we get.
> > >
> > > -Michal
> > >
> > >
> > > On Fri, Jun 6, 2014 at 11:36 AM, Brian LeRoux <b@brian.io> 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 <mmocny@chromium.org>
> > > wrote:
> > > >
> > > > > On Thu, Jun 5, 2014 at 6:47 PM, Brian LeRoux <b@brian.io> 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
> > > > >
> > > >
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message