PATCH: remove process.mixin

[Isaac Schlueter]

As discussed in IRC this morning, process.mixin has a few major issues.

Deep object copying is something that belongs in userland, not in
src/node.js. These patches eliminate it.

(In my opinion, thorough robust deep-copy-and-merge functionality
would be a great thing to have in a lib/object.js module or something,
but it has nothing to do with the *process* and thus doesn't belong in
node.js or on the process object.)

See attachments, or http://github.com/isaacs/node/commits/mixin-deprecation

--i

[Marco Rogers]

Would you say the same thing about sys.inherits? It's a convenience
method for prototypal inheritance. It's not documented but I stumbled
across it in the http lib code. I started using it and it works just
fine, but I would echo your argument that it belongs in a different
namespace than sys.

:Marco

>  0001-Remove-process.mixin-dependency-from-all-tests.patch
> 41KViewDownload
>
>  0002-Remove-process.mixin-dependency-from-fs.patch
> 1KViewDownload
>
>  0003-Remove-process.mixin-dependencies-from-benchmark-scr.patch
> 2KViewDownload
>
>  0004-Remove-process.mixin-from-repl.patch
> < 1KViewDownload
>
>  0005-Change-the-include-message-so-that-it-doesn-t-recomm.patch
> 1KViewDownload
>
>  0006-Remove-process.mixin-references-from-the-documentati.patch
> 2KViewDownload
>
>  0007-Deprecate-process.mixin-printing-a-warning-to-stderr.patch
> 1KViewDownload

[Ryan Dahl]

Thanks Issac, I took patches 1-5. I think it ought to be moved to sys
for now. (Which is where I'm putting all the utility functions.)

[Marak Squires]

Errrr, fuck.

would be a great thing to have in a lib/object.js module

Yeah can we get started on this yesterday? I was using a lot of process.mixin() earlier on, then it broke a bunch, then kinda worked, then broke a bunch again. I understand the test coverage for deep-copy was never quite there.

Are there any current modules out there that can do stuff like this?

--
You received this message because you are subscribed to the Google Groups "nodejs" group.
To post to this group, send email to nod...@googlegroups.com.
To unsubscribe from this group, send email to nodejs+un...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/nodejs?hl=en.

[inimino]

On 2010-03-09 10:16, Marak Squires wrote:
> *Yeah can we get started on this yesterday? I was using a lot of

> process.mixin() earlier on, then it broke a bunch, then kinda worked, then
> broke a bunch again. I understand the test coverage for deep-copy was never
> quite there.
>
> Are there any current modules out there that can do stuff like this?

The original process.mixin was basically a cut-and-paste from jQuery,
where it is called 'extend'.

If you want similar functionality, find it in the library of your
choice and copy it into your project (or write your own if you feel
so inclined).

--
http://inimino.org/~inimino/blog/

[Kris Kowal]

On Tue, Mar 9, 2010 at 9:16 AM, Marak Squires <marak....@gmail.com> wrote:
> Errrr, fuck.
>
> would be a great thing to have in a lib/object.js module
>
> Yeah can we get started on this yesterday? I was using a lot of
> process.mixin() earlier on, then it broke a bunch, then kinda worked, then
> broke a bunch again. I understand the test coverage for deep-copy was never
> quite there.
>
> Are there any current modules out there that can do stuff like this?

I did a util module that has been furnished with a deepCopy method.
This should work in any implementation; Tom and I are working on
creating a narwhal-util CommonJS package that has just CommonJS
components that can run on other engines, and this would certainly be
one of the modules. Also term.js for terminal coloring streams, and a
bunch of others. In any case, Narwhal's lib directory works on Node
if you put it at the end of the require.paths, so it can be used as a
package without buying into its bootstrapping, although milage will
vary if you use something that depends on one of the CommonJS draft
modules. "util" depends on nothing.

http://github.com/280north/narwhal/blob/master/lib/util.js

Kris Kowal

[Kris Kowal]

On Tue, Mar 9, 2010 at 12:50 PM, Kris Kowal <cowber...@gmail.com> wrote:
> I did a util module that has been furnished with a deepCopy method.

Pardon, also update, deepUpdate, complete, deepComplete and others.
deepUpdate is what you're looking for, I think.

Kris Kowal

[Ryan Dahl]

On Tue, Mar 9, 2010 at 12:50 PM, Kris Kowal <cowber...@gmail.com> wrote:

> I did a util module that has been furnished with a deepCopy method.
> This should work in any implementation; Tom and I are working on
> creating a narwhal-util CommonJS package that has just CommonJS
> components that can run on other engines, and this would certainly be
> one of the modules.  Also term.js for terminal coloring streams, and a
> bunch of others.  In any case, Narwhal's lib directory works on Node
> if you put it at the end of the require.paths, so it can be used as a
> package without buying into its bootstrapping, although milage will
> vary if you use something that depends on one of the CommonJS draft
> modules.  "util" depends on nothing.
>
> http://github.com/280north/narwhal/blob/master/lib/util.js
>
> Kris Kowal

Great! Ideally I could include this as a "dep"

[Jannick Knudsen]

Dear nodejs'ers,

Has anyone made a solid drop-in replacement for process.mixin?

Or is everybody just doing their own hacks?

/JJ

[Jannick]

Dear nodejs'ers

[inimino]

On 2010-03-15 08:39, Jannick wrote:
> Dear nodejs'ers
>
> Has anyone made a solid drop-in replacement for process.mixin?

In the vast majority of cases I've seen, people were using
process.mixin to copy the properties of one object onto
another object (a "shallow copy"), e.g. copying one exports
object's properties onto the global object (generally
regarded as a bad idea) or onto another exports object.

In such cases, all you need is four lines:

function extend(a,b){var prop
for(prop in b) if Object.prototype.hasOwnProperty.call(b,prop)
a[prop] = b[prop]
return a}

The name "mixin" seems to confuse a lot of people into
thinking this is something magical or has something to
do with class-based OOP, rather than the simple, generic
operation on objects it actually is.

It also supported a recursive "deep extend" version
of the above. If you need that you may be better off
writing it yourself so you can think about the various
edge cases that arise (what to do with RegExp objects,
arrays, etc) and make decisions that fit your use case.

Hope this helps!

--
http://inimino.org/~inimino/blog/

[Friedemann Altrock]

Just a side note, v8 knows Object.keys, so the following is
functionally equivalent:

function extend(a,b){var prop
for(prop in Object.keys(b))

a[prop] = b[prop]
return a}

--
Regards
Friedemann Altrock

[Tim Caswell]

Not really, prop would be the indexes of the keys array in that
case. A normal for..in on the object if sufficient, we control the
code so there is no need for the hasOwnProperty check. If some really
wants to extend Object.prototype we can do that safely with
Object.defineProperty. Just set the enumerable to false on the new
thing.

On Mar 15, 2:33 pm, Friedemann Altrock <froden...@googlemail.com>
wrote:

[inimino]

On 2010-03-15 13:47, Tim Caswell wrote:
> A normal for..in on the object if sufficient, we control the
> code so there is no need for the hasOwnProperty check.

People may want to use this on objects that have custom
prototype objects (i.e. objects that are instances of at least
one constructor other than Object), so the hasOwnProperty test
is a good idea if you're not trying to copy properties of a
user-created prototype object.

--
http://inimino.org/~inimino/blog/

[Jannick]

Thanks for the answers ... Im off to hack my own now then...

[Rasmus Andersson]

I'm using this simple version which covers all my needs:
http://github.com/rsms/oui/blob/master/oui/std-additions.js#L9

> --
> You received this message because you are subscribed to the Google Groups
> "nodejs" group.
> To post to this group, send email to nod...@googlegroups.com.
> To unsubscribe from this group, send email to
> nodejs+un...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/nodejs?hl=en.
>
>

--
Rasmus Andersson

[Anders Hellerup Madsen]

I use MooTools. The download builder at http://mootools.net/core can
be used to only pick the parts of the framework that doesn't mess with
the DOM, and I think that it's a very cool base library to work from.
If you just want a process.mixin() replacement, check Core, and
uncheck everything else. The functions $mixin() function does what
process.mixin() used to.

String, Array, Number, Function and Hash can also safely be added.
They mix in a lot of really nice convenience methods to the prototypes
of the basic objects.

If you wanted to, could also add Class and Class.Extras from the core-
builder, or snatch Date, Date.Extras and the Date.<language> packages
for some much needed proper JavaScript date/time support.

I'm not sure it would be a good idea to use this in library code, but
for applications this is a really neat package.

[Marco Rogers]

I finally got around to taking a look at both of the mixin
replacements suggested by Kris and Rasmus

http://github.com/rsms/oui/blob/master/oui/std-additions.js#L9
http://github.com/280north/narwhal/blob/master/lib/util.js

Found some things that might be considered issues with both.

Kris, deepUpdate works great except it doesn't check for Getters and
Setters. Instead it just ends up taking the value returned from any
Getter properties that are copied. And if the property has only a
Setter, the property name is created with a value of undefined.
Examples:

var o = {};
util.object.deepUpdate(o, {get foo() { return 'foo'; }});
// o => { foo: 'foo' }

var o = {};
util.object.deepUpdate(o, {set foo(v) { sys.puts('My new value is ' +
v); }});
// o => { foo: undefined }

The version in Rasmus's oui lib handles Getters and Setters, but
there's a small issue. The function only checks the Setter if the
Getter is also present.

var o = {};
mixin(o, {get foo() { return 'foo'; }});
// o => { foo: [Getter] }

var o = {};
mixin(o, {get foo() { return 'bar'; }, set foo(v) { sys.puts('My new
value is ' + v); }});
{ foo: [Getter/Setter] }

var o = {};
mixin(o, {set foo(v) { sys.puts('My new value is ' + v); }});
// o => { foo: undefined }

It's completely valid to have a write only property, so it should
probably copy both either way.

Now I say these MIGHT be issues because the validity of each approach
can be debated. copying getters/setters is certainly convenient on
it's face. But it could lead to issues. The getter could be hooked
into changing some complex internal state of the mixin object. That
also happens with regular functions though, so maybe that's moot. But
what about closures? It's probably okay to copy over a function that
references closure scope variables. But does that make sense for a
getter? Do getters even pick up closures? I assume so, but it's after
5:30am and I don't have the brain power to check :). Anyway, I'm
curious to hear people's thoughts on this.

One more small difference is that Rasmus's function has a small check
to prevent circular references (or at least that's how I'm reading
it).

...
else if (target !== d.value) {
target[k] = d.value
}

If the value of the mixin property is actually a reference to the
target object, don't do the mixin. Am I reading that right? Feels
like a good idea although I don't know how often that would come up.

Thanks for these functions guys. And the rest of the libraries as
well. They're both really useful, and they dropped into the
environment and worked no problem. However, Rasmus, I couldn't use
the npm package manager with yours because you don't provide a
package.json. Maybe on purpose because you don't expect that stuff to
be pulled out of oui. Just a heads up though.

:Marco

On Mar 16, 8:26 pm, Anders Hellerup Madsen

<anders.hellerup.mad...@gmail.com> wrote:
> I use MooTools. The download builder athttp://mootools.net/corecan
> be used to only pick the parts of the framework that doesn't mess with
> the DOM, and I think that it's a very cool base library to work from.

> If you just want aprocess.mixin() replacement, check Core, and
> uncheck everything else. The functions $mixin() function does whatprocess.mixin() used to.

[Jan Schütze]

Thanks for the detailed list ;).

I was watching this and another thread to see what's best as
replacement for my usecase: A prototype (BaseApplication) should be
extended with Logging functionality.
extend(true, BaseApplication.prototype, Logging.prototype,
AnotherAspect.prototype);

The extend method can be found here:
http://github.com/DracoBlue/spludo/blob/master/core/util.js#L79 , it's
basicly jQuery's extend method, without other dependencies (no
isArray, isPlainObject and so on necessary).

Regards,
Draco

--

http://dracoblue.net

[pavel zaitsev]

Looking at ruby side of things, I think getters and setters should be pushed into language/stdlib side of things and be dealt with separately. I shrink from idea of having beans in Javascript.
Just a thought,
Pavel

[Marco Rogers]

Does anyone else want to comment on this issue? Should getters/
setters be transfered by mixins? What are the potential issues? I'm
of the opposite opinion of pavel. I think getters/setters are part of
the behavior signature of the object. If methods get copied, these
should as well. I just want to know if anyone else has put thought
into it, or even run into real issues with this. I'm cross-posting to
commonjs as the more general questions is probably appropriate there.

:Marco

> > > > > nodejs+un...@googlegroups.com<nodejs%2Bunsu...@googlegroups.com>

> > .
> > > > > For more options, visit this group at
> > > > >http://groups.google.com/group/nodejs?hl=en.
>
> > > > --
> > > > Rasmus Andersson
>
> > --
> > You received this message because you are subscribed to the Google Groups
> > "nodejs" group.
> > To post to this group, send email to nod...@googlegroups.com.
> > To unsubscribe from this group, send email to

> > nodejs+un...@googlegroups.com<nodejs%2Bunsu...@googlegroups.com>

[Tim Caswell]

I think if you're doing a deep copy then the getters/setters should be cloned as is and not their value. I'm just not sure "mixin" is a good name for something doing a deep copy. That sounds like a shallow copy to me. All the cases where I used process.mixin I intended it to do a shallow copy. (of course, even for a shallow copy, I'd still prefer the getters/setters to reference the actual getter/setter and not their return value)

> To unsubscribe from this group, send email to nodejs+un...@googlegroups.com.

[Dean Landolt]

On Mon, Apr 5, 2010 at 1:21 PM, Tim Caswell <t...@creationix.com> wrote:

I think if you're doing a deep copy then the getters/setters should be cloned as is and not their value.  I'm just not sure "mixin" is a good name for something doing a deep copy.  That sounds like a shallow copy to me.  All the cases where I used process.mixin I intended it to do a shallow copy. (of course, even for a shallow copy, I'd still prefer the getters/setters to reference the actual getter/setter and not their return value)

Isn't process.mixin slated to go away? This is the kind of thing folks will certainly have various opinions about -- since it can all be done in pure js shouldn't it go the route of promises?

[Marco Rogers]

Isn't process.mixin slated to go away? This is the kind of thing folks will certainly have various opinions about -- since it can all be done in pure js shouldn't it go the route of promises?

tl:dr ;)  Dean, that's what this thread has essentially turned into.  We started discussing alternatives to process.mixin.  See my post above.  I evaluated a few that were submitted and ended up with questions about getters/setters.

:Marco

--
Marco Rogers
marco....@gmail.com

Life is ten percent what happens to you and ninety percent how you respond to it.
- Lou Holtz

[Tim Caswell]

Of course, since I work on HEAD mostly, it's already gone to me. I thought this thread was discussing what should go in the third-party replacement for it.

On Apr 5, 2010, at 12:30 PM, Dean Landolt wrote:

>
> Isn't process.mixin slated to go away? This is the kind of thing folks will certainly have various opinions about -- since it can all be done in pure js shouldn't it go the route of promises?
>

[Dean Landolt]

On Mon, Apr 5, 2010 at 1:38 PM, Tim Caswell <t...@creationix.com> wrote:

Of course, since I work on HEAD mostly, it's already gone to me.  I thought this thread was discussing what should go in the third-party replacement for it.

Appologies for not going back and rereading the whole thread -- I already forgot the bits I read two days ago :)

[Matthew Ranney]

On Mon, Apr 5, 2010 at 10:18 AM, Marco Rogers <marco....@gmail.com> wrote:

Does anyone else want to comment on this issue?  Should getters/
setters be transfered by mixins?  What are the potential issues?  I'm
of the opposite opinion of pavel.  I think getters/setters are part of
the behavior signature of the object.  If methods get copied, these
should as well.  I just want to know if anyone else has put thought
into it, or even run into real issues with this.  I'm cross-posting to
commonjs as the more general questions is probably appropriate there.

Copying getters and setters is certainly something you'd expect to work at first, but I think it'll end up being more confusing than useful.  How deeply do you copy the getter/setter and all of its references?

var sys = require('sys'),

    a, b;

    

a = (function () {

    var foo = 129;

        

    return {

        get get_foo () {

            return foo;

        }

    };

}());

b = {

    foo: 123

};

process.mixin(b, a);

sys.puts(a.get_foo + " " + b.get_foo);

// 129 129

[Marco Rogers]

That's definitely what I expected to happen with the closures.  Thanks for that quick confirmation.  And the way closures and getters/setters are defined, I say that's how it SHOULD work.  But there's definitely some ammo to shoot yourself in the foot.

Not to mention the potential for memory leaks. I'm just learning about the v8 internals, so this might not be a big deal.  But in this example, if you wrap the wrong thing from object a in a closure, how much of that object is gonna stick around and for how long?

I still say Rasmus has the right idea though. I'll be sticking with his implementation for now.

:Marco

--
You received this message because you are subscribed to the Google Groups "nodejs" group.
To post to this group, send email to nod...@googlegroups.com.

To unsubscribe from this group, send email to nodejs+un...@googlegroups.com.

For more options, visit this group at http://groups.google.com/group/nodejs?hl=en.

--