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

Ensure static private method init run before static property #12918

Merged
merged 3 commits into from Mar 3, 2021

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Feb 26, 2021

Q                       A
Fixed Issues? Fixes #12905
Patch: Bug Fix? Y
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

This PR is based on #12917, please review that one first.

As suggested in #12905, this PR places private methods before private static properties, see also the commit message for the rationale.

Per 15.7.12 Runtime Semantics: ClassDefinitionEvaluation, when DefineField is invoked, the class binding used in field initializer will have a receiver F, which has all static methods defined but may still have uninvoked initializer due to the order of DefineField executions on staticFields. Therefore, we place static class methods definition before static field.

For example, the following snippet

class C {
  static #p = 1;
  static get #m() {}
}
currently transforms to
"use strict";

class C {}

var _get_m = function () {};

var _p = {
  writable: true,
  value: 1
};
var _m = {
  get: _get_m,
  set: void 0
};
new C();
in this PR transforms to
"use strict";

class C {}

var _get_m = function () {};

var _m = {
  get: _get_m,
  set: void 0
};
var _p = {
  writable: true,
  value: 1
};
new C();

where _m is placed before _p, so when _p.value is evaluated, the static method descriptor _m will have been defined.

@JLHwung JLHwung added PR: Bug Fix 🐛 A type of pull request used for our changelog categories Spec: Private Methods labels Feb 26, 2021
@babel-bot
Copy link
Collaborator

babel-bot commented Feb 26, 2021

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/43651/

@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 26, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 4ca3404:

Sandbox Source
babel-repl-custom-plugin Configuration
babel-plugin-multi-config Configuration

@JLHwung JLHwung force-pushed the fix-12905 branch 3 times, most recently from e04868e to 9336747 Compare March 3, 2021 20:16
Per 15.7.12 Runtime Semantics: ClassDefinitionEvaluation, when DefineField is invoked, the class binding used in field initializer will have a receiver F, which has all static methods defined but may still have uninvoked initializer due to the order of DefineField executions on staticFields. Therefore, we place static class methods definition before static field.
@nicolo-ribaudo nicolo-ribaudo merged commit ac758f7 into babel:main Mar 3, 2021
@nicolo-ribaudo nicolo-ribaudo deleted the fix-12905 branch March 3, 2021 23:02
This was referenced Mar 17, 2021
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jun 3, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Bug Fix 🐛 A type of pull request used for our changelog categories Spec: Private Methods
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using a set/get accessor causes a runtime error if the descriptor is not yet initialized
4 participants