cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Shazron <shaz...@gmail.com>
Subject Re: [Node 101] Part 1: Small Modules
Date Mon, 16 Dec 2013 17:49:50 GMT
I've encountered this myself in personal projects. With tests you will
catch it early of course, but I'm not sure if there is a linter/tool that
will catch this early on to save grief later....


On Mon, Dec 16, 2013 at 4:49 AM, Patrick Mueller <pmuellr@gmail.com> wrote:

> What I've learned from this is to be careful when you use `module.exports
> =`.
>
> It doesn't come up much, because it's only a problem (I think) when there
> is some mutual recursion in your require() trees.  I've done this to myself
> when building packages where I was exporting functions via `module.exports
> =` as a common pattern, and then "crossed the paths" somewhere.
>
> Note that this is a dev-time problem, and not a run-time problem.  Assuming
> you are testing all your code, you will find problems with this pattern
> VERY EARLY.  So, it's nothing to be scared of or outlaw entirely.  It's
> never going to "pop up" unexpectedly at runtime.
>
> It is, unfortunately, a bit of a mysterious bug, when it does happen.  So I
> wanted others to be aware of this one weird problem with the pattern.
>
>
>
> On Sun, Dec 15, 2013 at 8:08 PM, Brian LeRoux <b@brian.io> wrote:
>
> > Super interesting, I am surprised this doesn't come up more often really.
> >
> > I suppose the moral of the story is to avoid executing code in your
> modules
> > outside of the scope of things you are exporting.
> >
> >
> > On Mon, Dec 16, 2013 at 3:46 AM, Patrick Mueller <pmuellr@gmail.com>
> > wrote:
> >
> > > The big problem with exporting functions - or exporting anything by
> using
> > > `module.exports =` style of exporting - is recursive require problem.
>  I
> > > couldn't quickly find any ref to this on the web, so did a little gist:
> > > https://gist.github.com/pmuellr/7975152
> > >
> > >
> > > On Thu, Dec 12, 2013 at 7:37 PM, Brian LeRoux <b@brian.io> wrote:
> > >
> > > > Oh interesting. I can see what you mean though I think the only
> thing I
> > > > should know about the output is the return value. (In your case,
> just a
> > > > test for 'done'.) The behavior of the module is super important of
> > > course.
> > > > In the small modules philosophy the foo, bar.method, and baz would
> have
> > > > thier own tests for interface/output to satisfy the case you
> describe.
> > > >
> > > > Again, IT DEPENDS! The rimraf example is a perfect case for what you
> > > > describe.
> > > >
> > > >
> > > > On Fri, Dec 13, 2013 at 10:59 AM, Gord Tanner <gtanner@gmail.com>
> > wrote:
> > > >
> > > > > It depends what you define as outputs.
> > > > >
> > > > > in a given module:
> > > > >
> > > > > var foo = require('foo'), bar = require('bar'), baz =
> require('baz');
> > > > >
> > > > > module.exports = function(a, b) {
> > > > >     foo(3);
> > > > >     bar.method(a);
> > > > >     baz(b);
> > > > >
> > > > >    return "done";
> > > > > }
> > > > >
> > > > > I have always counted the calls to foo, bar and baz as output that
> > > needs
> > > > to
> > > > > be tested.  This would produce a spec like:
> > > > >
> > > > > "when calling this module":
> > > > >    "it calls foo with 3"
> > > > >    "it calls bar.method with a"
> > > > >    "it calls baz with b"
> > > > >    "it returns done"
> > > > >
> > > > > It is just easier to mock bar.method then foo
> > > > >
> > > > > ie:
> > > > >
> > > > > var rewire = require('rewire');
> > > > > var example = rewire('example');     //NOTE: rewire rather then
> > require
> > > > > example.__set__('foo', jasmine.createSpy());
> > > > >
> > > > > vrs:
> > > > >
> > > > > var example = require('example');
> > > > > var bar = require('bar');
> > > > > spyOn(bar, "method");
> > > > >
> > > > > I came across this problem when using one of Isaac's modules
> (rimraf
> > > [1])
> > > > > where I obviously didn't want to call that in a unit test from my
> > > module
> > > > > but I need to mock it out.  Rewire was the only way I could.
> > > > >
> > > > > [1] - https://github.com/isaacs/rimraf
> > > > >
> > > > >
> > > > >
> > > > > On Thu, Dec 12, 2013 at 6:37 PM, Brian LeRoux <b@brian.io>
wrote:
> > > > >
> > > > > > ALSO: lets avoid using terms like 'I agree' or 'I disagree'.
Its
> > > > > > programming. The answer is ALWAYS 'it depends'. No absolutes
in
> the
> > > sea
> > > > > of
> > > > > > change.
> > > > > >
> > > > > >
> > > > > > On Fri, Dec 13, 2013 at 10:34 AM, Brian LeRoux <b@brian.io>
> wrote:
> > > > > >
> > > > > > > Maybe. Have a look at Substack's code and you'll see he
has no
> > > > trouble
> > > > > > > testing. The reason being he tests interfaces and outputs
> instead
> > > of
> > > > > > > implementations. That will be another Node 101!
> > > > > > >
> > > > > > >
> > > > > > > On Fri, Dec 13, 2013 at 10:24 AM, Gord Tanner <
> gtanner@gmail.com
> > >
> > > > > wrote:
> > > > > > >
> > > > > > >> I also agree with this except for returning a function
from
> > > > > > >> module.exports.
> > > > > > >>
> > > > > > >> It is possible but makes mocking much much harder for
testing.
> > > > > > >>
> > > > > > >> think of:
> > > > > > >>
> > > > > > >>
> > > > > > >> var foo = require('foo');
> > > > > > >>
> > > > > > >> module.exports = {
> > > > > > >>     awesome: function (a) {
> > > > > > >>         foo(a+1);
> > > > > > >>    }
> > > > > > >> };
> > > > > > >>
> > > > > > >> It is kind of awkward to test this module's use of
the foo
> > module.
> > > >  It
> > > > > > can
> > > > > > >> be done with rewire [1] but is a little awkward.
> > > > > > >>
> > > > > > >> If foo was designed where it exported an object literal
with
> > > > functions
> > > > > > it
> > > > > > >> would be much easier to mock:
> > > > > > >>
> > > > > > >> var foo = require('foo');
> > > > > > >>
> > > > > > >> module.exports = {
> > > > > > >>     awesome: function (a) {
> > > > > > >>         foo.bar(a+1);
> > > > > > >>    }
> > > > > > >> };
> > > > > > >>
> > > > > > >> it("calls foo.bar", function () {
> > > > > > >>     var foo = require('foo');
> > > > > > >>     spyOn(foo, "bar");
> > > > > > >> });
> > > > > > >>
> > > > > > >> Just my 2 cents from a testing perspective.
> > > > > > >>
> > > > > > >>
> > > > > > >> [1] - https://github.com/jhnns/rewire
> > > > > > >>
> > > > > > >>
> > > > > > >> On Thu, Dec 12, 2013 at 6:06 PM, Brian LeRoux <b@brian.io>
> > wrote:
> > > > > > >>
> > > > > > >> > Create modules that are the smallest possible
unit of code.
> > Less
> > > > > code
> > > > > > is
> > > > > > >> > fast code. Faster to write. Faster to maintain.
Faster to
> > test.
> > > On
> > > > > the
> > > > > > >> > extreme end characters in the Node community such
as
> Substack
> > > > > > advocate a
> > > > > > >> > single function per module definition.
> > > > > > >> >
> > > > > > >> > module.exports = function() {
> > > > > > >> >   // my logic here
> > > > > > >> > }
> > > > > > >> >
> > > > > > >> > This is kind of extreme and not always possible
but a good
> > > > practice
> > > > > > >> > nonetheless. The idea is not new. Its a part of
the UNIX
> > > > philosophy:
> > > > > > "do
> > > > > > >> > one thing well" coined by Doug Mcilroy. [1]
> > > > > > >> >
> > > > > > >> > It can help you make code that looks like this
[2] into this
> > > [3].
> > > > > > >> >
> > > > > > >> >
> > > > > > >> >
> > > > > > >> > [1]
> > > > > >
> http://homepage.cs.uri.edu/~thenry/resources/unix_art/ch01s06.html
> > > > > > >> > [2]
> > > > > > >> >
> > > > > > >> >
> > > > > > >>
> > > > > >
> > > > >
> > > >
> > >
> >
> https://github.com/apache/cordova-js/blob/c320378b484a172a02d3ee26634bcc584f43b939/Gruntfile.js
> > > > > > >> > [3]
> > > https://github.com/apache/cordova-js/blob/master/Gruntfile.js
> > > > > > >> >
> > > > > > >>
> > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> > >
> > >
> > > --
> > > Patrick Mueller
> > > http://muellerware.org
> > >
> >
>
>
>
> --
> Patrick Mueller
> http://muellerware.org
>

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