Move new private elements helper parameters #16299
Conversation
| setter: Function, | ||
| receiver: any, |
There was a problem hiding this comment.
If we swap these two parameters it will probably gzip slightly better, because privateMap, receiver will be next to each other both times.
There was a problem hiding this comment.
Yes, just because setter may be bind. :)
Do you have any example? |
| [_init_a, _get_a, _set_a, _init_b, _get_b, _set_b, _initProto] = babelHelpers.applyDecs(_Foo, [[dec, 1, "a", function () { | ||
| return babelHelpers.classPrivateFieldGet2(this, _A); | ||
| return babelHelpers.classPrivateFieldGet2(_A, this); | ||
| }, function (value) { | ||
| babelHelpers.classPrivateFieldSet2(this, _A, value); | ||
| babelHelpers.classPrivateFieldSet2(_A, this, value); | ||
| }], [dec, 1, "b", function () { | ||
| return babelHelpers.classPrivateFieldGet2(this, _B); | ||
| return babelHelpers.classPrivateFieldGet2(_B, this); | ||
| }, function (value) { | ||
| babelHelpers.classPrivateFieldSet2(this, _B, value); | ||
| babelHelpers.classPrivateFieldSet2(_B, this, value); | ||
| }]], []); |
There was a problem hiding this comment.
I want to try to optimize them to avoid an extra function.
It's a little difficult to be honest, but worth trying. :)
[_init_a, _get_a, _set_a, _init_b, _get_b, _set_b, _initProto] = babelHelpers.applyDecs(_Foo, [[dec, 1, "a",
bind(babelHelpers.classPrivateFieldGet2, _A),
bind(babelHelpers.classPrivateFieldSet2, _A)], [dec, 1, "b",
bind(babelHelpers.classPrivateFieldGet2, _B),
bind(babelHelpers.classPrivateFieldSet2, _B)]], []);| function _get_privateFieldValue(_this) { | ||
| return babelHelpers.classPrivateFieldGet2(_privateField, _this); | ||
| } | ||
| function _set_privateFieldValue(newValue) { | ||
| babelHelpers.classPrivateFieldSet2(this, _privateField, newValue); | ||
| function _set_privateFieldValue(_this2, newValue) { | ||
| babelHelpers.classPrivateFieldSet2(_privateField, _this2, newValue); | ||
| } |
There was a problem hiding this comment.
I pushed another commit and now they can bind _privateField to save some size.
If you want to keep this PR easy to review I can revert it and open a new PR. )
I also need to handle arguments. :)
There was a problem hiding this comment.
I like this change -- passing this as a parameter also makes minification better because it can be renamed.
There was a problem hiding this comment.
I am doing a special case for arguments. :)
0646bb4 to
774e467
Compare
ed33d4b to
f75460c
Compare
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/56348 |
| var _set_privateFieldValue = babelHelpers.curryThis(function _set_privateFieldValue(newValue) { | ||
| babelHelpers.classPrivateFieldSet2(_privateField, this, arguments[0]); | ||
| }); |
There was a problem hiding this comment.
In this case, the output becomes worse, but I think this is an extreme situation.
|
@liuxingbaoyu I was going to merge this PR because I have another PR to open on top of it -- could you remove the last commit and push it in a new PR to give time to review it? 🙏 |
690bb82 to
c224850
Compare
|
@nicolo-ribaudo Sorry, I seem to have done something wrong. There is still the second Commit in this PR, I don't understand why. 😞
Oh, what you mean is that there is only the last commit, so this may not matter. : :) |
|
Yes sorry, I re-pushed it when merging 😅 But today and yesterday GitHub has been having problems with showing pushes in PRs. |
Fixes #1, Fixes #2This PR itself is not an improvement, but it opens up the possibility for future output size improvements.
I also migrated the helpers to ts so that we can use vscode's reference analysis.