Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable clone to be extended. #108

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

btsimonh
Copy link

replaces #93 - but allows for the same kind of global modification of clone behaviour.

add clone.globalopts containing _clone.
If set, globalopts._clone is called with each item at the start of _clone().
If it returns undefined, then the normal _clone() continues.
If it returns something, the result is is used as the item being cloned, and the rest of _clone() is skipped.

ToDo:

  • Add a meaningful entry to the changelog in the README.md.
  • You can add yourself to the list of contributors in the package.json if you
    like. If you don't do it, you won't be mentioned.

Thanks!

…ach item. If it returns undefined, then the normal _clone continues. If it returns something, this is used as the item being cloned.
@btsimonh
Copy link
Author

ref the end of #106 :

this mod may be demonstrated using the below code:

// get a first copy of clone
var clone = require('clone');
clone.clone1 = true;

// get a second copy of clone
var clone2 = require('clone');

// setup for a fail by spawning a process to get a ChildProcess object
var cp = require('child_process');
var spawn = cp.spawn;
var child = spawn('cmd', [], {});

// the object we will try to clone.
var obj = {
  child: child,
  teststruct: {
    willchange: 1,
  }
};

// simplest example
clone.globalopts._clone = function(parent, depth){
  if (parent._pleasedontcopyme){
    return parent;
  } else {
    return;
  }
}


// tell the globalopts._clone function that the ChildProcess object should be copied by reference, not cloned.
obj.child._pleasedontcopyme = true;

// should succeed
console.log("will succeed - but is a ref copy, not a clone");
var copy = clone(obj);
obj.teststruct.willchange = 2;
console.log("obj.teststruct.willchange should be 1");
console.log(copy);
delete obj.child._pleasedontcopyme;


// complex example
clone.globalopts._clone = function(parent, depth){
  if (parent._clone && (typeof parent._clone === 'function')) {
    return parent._clone(parent, depth);
  } else {
    return;
  }
}

// completely replace the object with something else
obj.child._clone = function(parent, depth){
  return { message: "object could not be cloned" };
}

// test that the globalopts structure applies to our second clone require:
console.log("will succeed - but child is replaced, not a clone");
var copy1a = clone2(obj);
obj.teststruct.willchange = 18;
console.log("obj.teststruct.willchange should be 2, not 18");
console.log(copy1a);

// may kill nodejs/V8
// remove patch from clone:
clone.globalopts._clone = null;

console.log("may kill nodejs/V8 - because cloning this type of object is not supported");
var copy2 = clone(obj);
obj.teststruct.willchange = 3;
console.log("obj.teststruct.willchange should be 1");
console.log(copy2);

@btsimonh
Copy link
Author

@pvorb - if uyou like this PR, then just say and I'll add to the readme.
Are checks already broken, because I don't see how this change can have caused the failure it's stating?

@btsimonh
Copy link
Author

The V8 crash protection is based on a simple list of '[object Type]' strings, and is in the extensible portion, so anyone can enhance the protection as more types are discovered that can't be cloned.
e.g. from user code,
clone.globalopts.crash_types['[object Socket]'] = true;
would make Sockets be referenced, not cloned.

or indeed, in my case
clone.globalopts.crash_types['object ChildProcess'] = true;

  • which may be a more appropriate way of dealing with my immediate issue! (but I still would like extensibility :) ).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants