-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Improve output when privateFieldsAsProperties
or privateFieldsAsSymbols
#16313
base: main
Are you sure you want to change the base?
Improve output when privateFieldsAsProperties
or privateFieldsAsSymbols
#16313
Conversation
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/56537 |
30d7bd8
to
9c4e868
Compare
c1f929e
to
b7ae29b
Compare
@@ -893,6 +893,15 @@ helpers.classPrivateFieldLooseBase = helper("7.0.0-beta.0")` | |||
} | |||
`; | |||
|
|||
helpers.classPrivateFieldGetLoose = helper("7.24.0")` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you move it to a standalone file? :)
babelHelpers.classPrivateFieldLooseBase(this, _foo)[_foo] = 2; | ||
babelHelpers.classPrivateFieldLooseBase(other.obj, _foo)[_foo] += 1; | ||
babelHelpers.classPrivateFieldLooseBase(other.obj, _foo)[_foo] = 2; | ||
babelHelpers.classPrivateFieldGetLoose(this, _foo, 1)[_foo] += 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this a size increase in general? It looks shorter because the helepr name is shorter, but once you minify the helper name to 1 char this has ,1
more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can pass ,1
to mean "return the property" rather than "return the receiver", so that we are sure that it's an improbement rather than depending on the ratio of get to set.
@@ -839,6 +839,13 @@ const privateNameHandlerLoose: Handler<PrivateNameState> = { | |||
const { object } = member.node; | |||
const { name } = (member.node.property as t.PrivateName).id; | |||
|
|||
if (process.env.BABEL_8_BREAKING || newHelpers(file)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
newHelpers
doesn't work here anymore, because it returns true in 7.24.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah. I have modified newHelpers
to return true in v7.24.1. Maybe the changes were lost for some reason.
What do you think of this approach?
@@ -3,7 +3,7 @@ var _privateStaticMethod = /*#__PURE__*/babelHelpers.classPrivateFieldLooseKey(" | |||
class Cl { | |||
constructor() { | |||
if (exfiltrated === undefined) { | |||
exfiltrated = babelHelpers.classPrivateFieldLooseBase(Cl, _privateStaticMethod)[_privateStaticMethod]; | |||
exfiltrated = babelHelpers.classPrivateFieldGetLoose(Cl, _privateStaticMethod); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For methods, when the receiver is the class name, we can skip the helper call (similarly to how we do for the non-loose transform)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
b7ae29b
to
c701ff0
Compare
c701ff0
to
b6821f7
Compare
b6821f7
to
005dd59
Compare
Fixes #1, Fixes #2
attempted to use private field on non-instance
is now replaced by throwingTypeError: this[_privateMethod] is not a function
, which I think does not matter since this is not inspec
mode.Contains #16312, please only view the last commit, it does not require blocking release.