Jeff-Lewis / cls-hooked

cls-hooked : CLS using AsynWrap or async_hooks instead of async-listener for node 4.7+
BSD 2-Clause "Simplified" License
758 stars 89 forks source link

Correctly clone existing active context #43

Open ashleyw opened 5 years ago

ashleyw commented 5 years ago

Hi,

From what I understand, nested contexts are permitted and values will be inherited, however within createContext it currently uses Object.create(), but that doesn't actually clone the active context (on Node v8.16.0 & v10.16.0):

let context = Object.create(this.active ? this.active : Object.prototype);

Where I believe it should be using Object.assign() to clone the object:

const context = this.active ? Object.assign({}, this.active) : {};

For example:

const active = { _ns_name: '__ictx__', id: 145, a: -1, b: 2, c: 3, d: 4 };

Object.create(active);
// => {}

Whereas:

const active = { _ns_name: '__ictx__', id: 145, a: -1, b: 2, c: 3, d: 4 };

Object.assign({}, active);
// => { _ns_name: '__ictx__', id: 145, a: -1, b: 2, c: 3, d: 4 }

Am I missing something, or is this a bug?

Thanks!

sbuljac commented 4 years ago

@ashleyw Object.create will set this.active as a prototype of a newly created object context. When a property is not found directly on context it will be searched on its prototype, in this case, this.active.

const prot = { test: 5 };
const obj = Object.create(prot);
console.log(obj.test)
// 5