zlib: fix inheritance of DeflateRaw without classes #13370
Conversation
nodejs-github-bot
added
the
util
|
Change looks good, testing it really quick locally before signing off.. |
lib/internal/util.js
Outdated
| return Reflect.construct(type, args, new.target || type); | ||
| const res = Reflect.construct(type, args, new.target || type); | ||
|
|
||
| if (this) { |
There was a problem hiding this comment.
if (this && !new.target)? Otherwise this would also set the prototype for cases like new Class(), right?
|
(I stopped the previous CI job) new CI: https://ci.nodejs.org/job/node-test-pull-request/8399/ |
|
do you think the commit message is ok? or it should be tagged as |
|
there was a bug in my test, and it is not working as expected, i.e. it is not fixing the bug :(. |
|
I'm running a CITGM on this, it seems a little "breaky" (explicitly calling |
Fixes internal/util createClassWrapper to support inheritance without using classes. The constructor now needs to be defined using a Symbol. Fixes: nodejs#13358
|
Eh, yes, this is a bit icky. The code still LGTM but I think I might prefer just going back to using functions instead of classes instead… |
|
I just checked it quickly and it works in |
|
The fix is a bit unfortunate but it definitely makes sense in that this approach does essentially the same thing as the old function approach. @mcollina is away at the moment but I'm going through and seeing if there's a way of making the fix cleaner |
|
In general... I think reverting back to using Functions is likely going to be the best bet long term... it is unfortunate, and is not going to be a clean revert... I'll work on that today |
|
I've opened #13374 as an alternative. |
Using ES6 Classes broke userland code. Revert back to functions. Fixes: nodejs#13358 Refs: nodejs#13370
|
Closing in favor of #13374 which just landed. |
Using ES6 Classes broke userland code. Revert back to functions. PR-URL: #13374 Fixes: #13358 Ref: #13370 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Daijiro Wachi <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
Using ES6 Classes broke userland code. Revert back to functions. PR-URL: #13374 Fixes: #13358 Ref: #13370 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Daijiro Wachi <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
Fixes internal/util createClassWrapper to support inheritance
without using classes.
Fixes: #13358
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
zlib