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

Correctly clone existing active context #43

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Correctly clone existing active context #43

wants to merge 1 commit into from

Conversation

ashleyw
Copy link

@ashleyw ashleyw commented Jun 3, 2019

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
Copy link

sbuljac commented Oct 23, 2019

@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

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.

None yet

2 participants