Add typings to `create-class-features-plugin` helper by colinaaa · Pull Request #13570 · babel/babel

colinaaa · 2021-07-17T16:28:38Z

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

I was trying to read codes in babel-helper-create-class-features-plugin, then I found that most of its code are written without typings. So I tried to add the correct typings to it, hopefully it can help others to read the code easier.

babel-bot · 2021-07-17T16:32:46Z

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

codesandbox-ci · 2021-07-17T16:37:26Z

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 0bc80ca:

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

packages/babel-helper-create-class-features-plugin/src/fields.ts

colinaaa · 2021-07-19T15:27:09Z

Hi @nicolo-ribaudo, I did not find anything about path.isPrivateMethod. Is this a typo to path.isClassPrivateMethod or it's something special?

babel/packages/babel-helper-create-class-features-plugin/src/features.ts

Lines 148 to 149 in aa18da0

    
           // NOTE: path.isPrivateMethod() it isn't supported in <7.2.0 
        
           if (path.isPrivateMethod?.()) {

nicolo-ribaudo · 2021-07-19T20:21:32Z

Yes it was a typo 😬

nicolo-ribaudo

Thanks for all these type improvements!

packages/babel-helper-create-class-features-plugin/src/decorators.ts

nicolo-ribaudo · 2021-07-19T20:29:05Z

packages/babel-helper-create-class-features-plugin/src/features.ts

+  let resolvedLoose: boolean;
+  let higherPriorityPluginName: string;


These can also be undefined.

would be nice to enable strict mode, while adding improving type coverage… - or at least strictNullChecks which will catch things like this

however, not sure if it can be easily done for single package… probably this would require to revert project references configuration and fix circular dependencies…

nicolo-ribaudo · 2021-07-19T20:30:01Z

packages/babel-helper-create-class-features-plugin/src/features.ts

@@ -146,7 +150,7 @@ export function verifyUsedFeatures(path, file) {
  }

  // NOTE: path.isPrivateMethod() it isn't supported in <7.2.0


Nit: also update the comment here.

nicolo-ribaudo · 2021-07-19T20:32:20Z

packages/babel-helper-create-class-features-plugin/src/fields.ts

-const privateInVisitor = privateNameVisitorFactory({
+const privateInVisitor = privateNameVisitorFactory<{
+  classRef: t.Identifier;
+  file: unknown;


nicolo-ribaudo · 2021-07-19T20:34:14Z

packages/babel-helper-create-class-features-plugin/src/misc.ts

  for (const computedPath of computedPaths) {
    const computedKey = computedPath.get("key");
    if (computedKey.isReferencedIdentifier()) {
+      // @ts-expect-error TODO: make isReferencedIdentifier return path is Identifier


If you'll want to open a PR to do this, it will be a welcome improvement!

I will do that!

packages/babel-helper-member-expression-to-functions/src/index.ts

...babel-plugin-proposal-class-properties/test/fixtures/private/static-self-method/options.json

colinaaa · 2021-07-20T03:59:54Z

rebased to main to make CI pass

packages/babel-helper-member-expression-to-functions/src/index.ts

JLHwung · 2021-07-20T21:03:47Z

packages/babel-helper-replace-supers/src/index.ts

+      boundGet() {
+        // noop
+        // we dont need boundGet here, but memberExpressionToFunctions handler needs it.
+        throw new Error("");


Throwing exception is definitely not noop. I am good with either make it an noop or throw an explicit error here.

JLHwung · 2021-07-20T21:09:06Z

packages/babel-traverse/src/index.ts

 import type { Visitor } from "./types";

-export type { Visitor };
+export type { Visitor, Binding };


Q for team: Is adding new type exports considered new features?

Well, we strip away types before publishing so no (at least, for now).

packages/babel-helper-create-class-features-plugin/src/decorators.ts

packages/babel-helper-create-class-features-plugin/src/misc.ts

packages/babel-helper-create-class-features-plugin/src/features.ts

packages/babel-helper-create-class-features-plugin/src/misc.ts

fedeci · 2021-07-27T18:17:43Z

packages/babel-helper-member-expression-to-functions/src/index.ts

+
+export interface HandlerState<State = {}> extends Handler<State> {
+  handle(
+    this: HandlerState<State> & State,


Q: Are these circular references good for TS?

It seems that TS works pretty fine with these circular refs. Are there any better solutions?

Not sure there is any🤔

packages/babel-helper-create-class-features-plugin/src/index.ts

packages/babel-helper-create-class-features-plugin/src/fields.ts

nicolo-ribaudo · 2021-07-28T19:49:50Z

I merged the path.get PR on main: if you rebase this one we can rely on it (if needed).

much more todo

try to fix ci

fix ci

use assertExpression instead of if-else Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>

also replace assert with type cast

add File type

fix typo and simplify if-else logic

rebased to main

nicolo-ribaudo

Thanks!

nicolo-ribaudo added the Flow -> TS Tracking repository migration from Flow to TS label Jul 17, 2021

nicolo-ribaudo reviewed Jul 17, 2021

View reviewed changes

packages/babel-helper-create-class-features-plugin/src/fields.ts Outdated Show resolved Hide resolved

packages/babel-helper-create-class-features-plugin/src/fields.ts Outdated Show resolved Hide resolved

colinaaa marked this pull request as ready for review July 19, 2021 05:06

nicolo-ribaudo reviewed Jul 19, 2021

View reviewed changes

colinaaa mentioned this pull request Jul 20, 2021

Make virtual-types assertions more specific #13576

Closed

1 task

JLHwung reviewed Jul 20, 2021

View reviewed changes

packages/babel-helper-member-expression-to-functions/src/index.ts Outdated Show resolved Hide resolved

JLHwung reviewed Jul 20, 2021

View reviewed changes

packages/babel-helper-member-expression-to-functions/src/index.ts Outdated Show resolved Hide resolved

JLHwung reviewed Jul 20, 2021

View reviewed changes

packages/babel-helper-member-expression-to-functions/src/index.ts Outdated Show resolved Hide resolved

JLHwung reviewed Jul 20, 2021

View reviewed changes

colinaaa mentioned this pull request Jul 21, 2021

Improve type definition of NodePath.get() #13587

Closed

1 task

fedeci reviewed Jul 27, 2021

View reviewed changes

colinaaa and others added 12 commits July 29, 2021 11:25

feat(typing): add typings to create class features plugin helper

c7fc4e0

much more todo

feat: more typings for member expression to function helper

d652352

fix: fix memberExpressionToFunctions usage type error

eecb7c9

fix: try to fix ci

18f6a8c

refactor: reduce temp variable

6accf19

feat: more typings on class helper

77801a5

fix: import type instead of directly import

7727bcb

try to fix ci

fix: use path.node.static instead of isStatic()

c83b1b7

fix ci

feat: more typings to class helper

e798a94

Update packages/babel-helper-member-expression-to-functions/src/index.ts

b13cbae

use assertExpression instead of if-else Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>

fix: fix typos and suggested changes

c8793d1

refactor: use node instead of path for better performance

2501bdb

also replace assert with type cast

colinaaa and others added 4 commits July 29, 2021 11:25

feat: remove used todo and ts-expect-error

7203842

feat: apply suggestions from code review

0cb7811

add File type

feat: apply suggestions from code review

ec9beba

fix typo and simplify if-else logic

refactor: use get("body.body") with improved typings

0bc80ca

rebased to main

JLHwung approved these changes Aug 2, 2021

View reviewed changes

nicolo-ribaudo approved these changes Aug 2, 2021

View reviewed changes

nicolo-ribaudo changed the title ~~Add typings to create class features plugin helper~~ Add typings to create-class-features-plugin helper Aug 2, 2021

nicolo-ribaudo merged commit 1960f23 into babel:main Aug 2, 2021

colinaaa mentioned this pull request Aug 3, 2021

Disallow reinitializing private elements #13601

Merged

github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Nov 2, 2021

github-actions bot locked as resolved and limited conversation to collaborators Nov 2, 2021

		let resolvedLoose: boolean;
		let higherPriorityPluginName: string;

		@@ -146,7 +150,7 @@ export function verifyUsedFeatures(path, file) {
		}

		// NOTE: path.isPrivateMethod() it isn't supported in <7.2.0

Uh oh!

Conversation

colinaaa commented Jul 17, 2021 • edited by gitpod-io bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

babel-bot commented Jul 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codesandbox-ci bot commented Jul 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

colinaaa commented Jul 19, 2021

Uh oh!

nicolo-ribaudo commented Jul 19, 2021

Uh oh!

nicolo-ribaudo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

colinaaa commented Jul 20, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JLHwung Jul 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nicolo-ribaudo commented Jul 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicolo-ribaudo left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

colinaaa commented Jul 17, 2021 •

edited by gitpod-io bot

Loading

babel-bot commented Jul 17, 2021 •

edited

Loading

codesandbox-ci bot commented Jul 17, 2021 •

edited

Loading

JLHwung Jul 20, 2021 •

edited

Loading

nicolo-ribaudo commented Jul 28, 2021 •

edited

Loading