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

add declare to class properties type annotations #12257

Merged
merged 2 commits into from Oct 27, 2020

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Oct 26, 2020

Q                       A
Fixed Issues? Fixes e2e errors in #12244
License MIT

Currently we have redundant initializer code unshifted in the constructors from type annotations. For example

constructor(hub: HubInterface, parent: Object) {
this.parent = parent;
this.hub = hub;
this.contexts = [];
this.data = null;
// this.shouldSkip = false; this.shouldStop = false; this.removed = false;
this._traverseFlags = 0;
this.state = null;
this.opts = null;
this.skipKeys = null;
this.parentPath = null;
this.context = null;
this.container = null;
this.listKey = null;
this.key = null;
this.node = null;
this.scope = null;
this.type = null;
}
parent: Object;
hub: HubInterface;
contexts: Array<TraversalContext>;
data: Object;
shouldSkip: boolean;
shouldStop: boolean;
removed: boolean;
state: any;
opts: ?Object;
_traverseFlags: number;
skipKeys: ?Object;
parentPath: ?NodePath;
context: TraversalContext;
container: ?Object | Array<Object>;
listKey: ?string;
key: ?string;
node: ?Object;
scope: Scope;
type: ?string;

is transformed as (See here for the full outputs)

constructor(hub, parent) {
    // ... ignored initializer code
    this.shouldSkip = void 0;
    this.shouldStop = void 0;
    this.removed = void 0;
    // ... ignored initializer code
    this._traverseFlags = void 0;
  }

this.shouldSkip = void 0; transformed from the property type annotations, is redundant because we have initialized it in constructors. What's more, it causes side effects if we skip proposal-class-properties when building Babel in #12244, in which it is transformed to

constructor(hub, parent) {
  this._traverseFlags = void 0;
}
removed;
get removed() {}
set removed(removed) {}

When these class property annotations were authored, flow didn't support declare class properties and they happen to be removed in the built artifacts via proposal-class-properties. This PR adds declare to these properties so it is future-safe (removed by flow-types) after we skip proposal-class-properties.

@JLHwung JLHwung added the PR: Internal 🏠 A type of pull request used for our changelog categories label Oct 26, 2020
@babel-bot
Copy link
Collaborator

babel-bot commented Oct 26, 2020

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

@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 26, 2020

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 19777ce:

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

@nicolo-ribaudo
Copy link
Member

I don't remember when I added that comment, but maybe this lets us do

babel/babel.config.js

Lines 107 to 108 in 2782a54

// TODO: Use @babel/preset-flow when
// https://github.com/babel/babel/issues/7233 is fixed
?

@JLHwung JLHwung merged commit ea2892f into babel:main Oct 27, 2020
@JLHwung JLHwung deleted the declare-class-properties-type-annotations branch October 27, 2020 14:05
@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 Jan 27, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 27, 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: Internal 🏠 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants