Skip to content

Cloning prototypes with getter#36

Merged
pvorb merged 2 commits intopvorb:masterfrom
purposeindustries:prototype-with-only-getter
Nov 26, 2014
Merged

Cloning prototypes with getter#36
pvorb merged 2 commits intopvorb:masterfrom
purposeindustries:prototype-with-only-getter

Conversation

@oroce
Copy link
Contributor

@oroce oroce commented Oct 6, 2014

We are using clone in our logger: https://github.com/purposeindustries/node-logify-logstash-transport/blob/master/index.js#L20

If we are trying to clone a http.ServerResponse instance the module will throw an exception, because headersSent property has only getter: https://github.com/joyent/node/blob/7c0419730b237dbfa0ec4e6fb33a99ff01825a8f/lib/_http_outgoing.js#L398-L402

The added code does the follow:

  1. Save prototype into proto variable if it has prototype or we can figure it out
  2. If it has prototype it will try to find out the descriptors of the property
  3. If the property has descriptors but it doesnt have setter we will move to next property (we can safely ignore setting the property on the child object because Object.create already created the getter on child.)

pvorb added a commit that referenced this pull request Nov 26, 2014
@pvorb pvorb merged commit 34db7b7 into pvorb:master Nov 26, 2014
@pvorb
Copy link
Owner

pvorb commented Nov 26, 2014

Thanks a lot for this PR. I had a busy time the last few weeks, so I didn't find the time to merge it.

@oroce
Copy link
Contributor Author

oroce commented Dec 1, 2014

haha i totally forgot i did this pr. thanks for merging it!

@pvorb
Copy link
Owner

pvorb commented Dec 1, 2014

Me too. Just stumbled upon it a few days ago.

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