Backport: repl: don't override all internal repl defaults - #9774
Conversation
Sorry, something went wrong.
This commit adds a function to test/common.js that allows additional global variables to be whitelisted in a test. PR-URL: nodejs#7826 Reviewed-By: James M Snell <jasnell@gmail.com>
The createInternalRepl() module accepts an options object as an argument. However, if one is provided, it overrides all of the default options. This commit applies the options object to the defaults, only changing the values that are explicitly set. PR-URL: nodejs#7826 Reviewed-By: James M Snell <jasnell@gmail.com>
| opts = null; | ||
| } | ||
| opts = opts || { | ||
| opts = util._extend({ |
There was a problem hiding this comment.
I'm curious why you wouldn't just use Object.assign here. Looking at the source of util._extend it looks to be identical. What am I missing?
Sorry, something went wrong.
There was a problem hiding this comment.
Object.assign() was shown to be slower in a few previous issues.
Sorry, something went wrong.
There was a problem hiding this comment.
If not in critical path, May be Object.assign() is ok enough.
Sorry, something went wrong.
There was a problem hiding this comment.
This is a backport, so I don't think it's a good idea to change things here.
Sorry, something went wrong.
There was a problem hiding this comment.
+1 If you have problem with implementation fix on master and it can come in another backport
Sorry, something went wrong.
There was a problem hiding this comment.
Thanks for the info. Just for the record, I asked out of curiosity - not because I expected it to change. I had no idea that Object.assign was slower. To see for myself, I ran some tests and also found that to be the case. Thanks for the clarification.
Sorry, something went wrong.
|
LGTM |
Sorry, something went wrong.
|
landed in 7b4268b...698bf2e |
Sorry, something went wrong.
This is a backport of #7826 to v4.
R= @thealphanerd