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

console: bind methods from the prototype chain in Console #24047

Closed
wants to merge 1 commit into from

Conversation

@joyeecheung
Copy link
Member

commented Nov 2, 2018

In 6223236 we made the console.Console function construct an object
with methods from Console.prototype bound to the instance, instead of
with methods found on the prototype chain to be bound on the instances,
thus breaking users overriding these methods when subclassing Console
because they are not expecting this function to construct a
namespace whose methods are not looked up from the prototype
chain.

This patch restores the previous behavior since it does not affect
the characteristics of the global console anyway. So after this patch,
the Console function still constructs normal objects with function
properties looked up from the prototype chain but bound to the
instances always.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
console: bind methods from the prototype chain in Console
In 6223236 we made the console.Console function construct an object
with methods from Console.prototype bound to the instance, instead of
with methods found on the prototype chain to be bound on the instances,
thus breaking users overriding these methods when subclassing Console
because they are not expecting this function to construct a
namespace whose methods are not looked up from the prototype
chain.

This patch restores the previous behavior since it does not affect
the characteristics of the global console anyway. So after this patch,
the Console function still constructs normal objects with function
properties looked up from the prototype chain but bound to the
instances always.
@joyeecheung

This comment has been minimized.

Copy link
Member Author

commented Nov 2, 2018

@jdalton
jdalton approved these changes Nov 2, 2018
@lpinca
lpinca approved these changes Nov 2, 2018
@joyeecheung

This comment has been minimized.

Copy link
Member Author

commented Nov 3, 2018

@cjihrig
cjihrig approved these changes Nov 4, 2018
@Trott

This comment has been minimized.

Copy link
Member

commented Nov 6, 2018

Landed in 04a291a

@Trott Trott closed this Nov 6, 2018

Trott added a commit to Trott/io.js that referenced this pull request Nov 6, 2018
console: bind methods from the prototype chain in Console
In 6223236 we made the console.Console function construct an object
with methods from Console.prototype bound to the instance, instead of
with methods found on the prototype chain to be bound on the instances,
thus breaking users overriding these methods when subclassing Console
because they are not expecting this function to construct a
namespace whose methods are not looked up from the prototype
chain.

This patch restores the previous behavior since it does not affect
the characteristics of the global console anyway. So after this patch,
the Console function still constructs normal objects with function
properties looked up from the prototype chain but bound to the
instances always.

PR-URL: nodejs#24047
Reviewed-By: John-David Dalton <john.david.dalton@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

@targos targos added this to Don't land (ever) in v11.x Nov 14, 2018

@joyeecheung joyeecheung referenced this pull request Jan 9, 2019
3 of 3 tasks complete

@BridgeAR BridgeAR removed this from Don't land (ever) in v11.x Jan 9, 2019

joyeecheung added a commit to joyeecheung/node that referenced this pull request Jan 9, 2019
console: bind methods from the prototype chain in Console
In 6223236 we made the console.Console function construct an object
with methods from Console.prototype bound to the instance, instead of
with methods found on the prototype chain to be bound on the instances,
thus breaking users overriding these methods when subclassing Console
because they are not expecting this function to construct a
namespace whose methods are not looked up from the prototype
chain.

This patch restores the previous behavior since it does not affect
the characteristics of the global console anyway. So after this patch,
the Console function still constructs normal objects with function
properties looked up from the prototype chain but bound to the
instances always.

PR-URL: nodejs#24047
Reviewed-By: John-David Dalton <john.david.dalton@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@joyeecheung joyeecheung referenced this pull request Jan 9, 2019
2 of 2 tasks complete
BridgeAR added a commit that referenced this pull request Jan 9, 2019
console: bind methods from the prototype chain in Console
In 6223236 we made the console.Console function construct an object
with methods from Console.prototype bound to the instance, instead of
with methods found on the prototype chain to be bound on the instances,
thus breaking users overriding these methods when subclassing Console
because they are not expecting this function to construct a
namespace whose methods are not looked up from the prototype
chain.

This patch restores the previous behavior since it does not affect
the characteristics of the global console anyway. So after this patch,
the Console function still constructs normal objects with function
properties looked up from the prototype chain but bound to the
instances always.

PR-URL: #24047
Reviewed-By: John-David Dalton <john.david.dalton@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@BridgeAR BridgeAR referenced this pull request Jan 16, 2019
BridgeAR added a commit to BridgeAR/node that referenced this pull request Jan 16, 2019
console: bind methods from the prototype chain in Console
In 6223236 we made the console.Console function construct an object
with methods from Console.prototype bound to the instance, instead of
with methods found on the prototype chain to be bound on the instances,
thus breaking users overriding these methods when subclassing Console
because they are not expecting this function to construct a
namespace whose methods are not looked up from the prototype
chain.

This patch restores the previous behavior since it does not affect
the characteristics of the global console anyway. So after this patch,
the Console function still constructs normal objects with function
properties looked up from the prototype chain but bound to the
instances always.

PR-URL: nodejs#24047
Reviewed-By: John-David Dalton <john.david.dalton@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins MylesBorins referenced this pull request Jan 24, 2019

@targos targos added this to Backported in v11.x Jan 30, 2019

@BethGriggs

This comment has been minimized.

Copy link
Member

commented Jun 25, 2019

@BridgeAR, should this be backported to v10.x?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
9 participants
You can’t perform that action at this time.