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

Improve @babel/generator typings #14644

Merged
merged 10 commits into from Jun 13, 2022

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Jun 6, 2022

Q                       A
License MIT

See #14601 for more info. This PR improves @babel/generator typings with small refactors for more effective type inference.

| "RegexLiteral"
| "RestProperty"
| "SpreadProperty"
>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now the type checker will throw if we add a new AST node without a printer method.

@babel-bot
Copy link
Collaborator

babel-bot commented Jun 6, 2022

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

@babel-bot
Copy link
Collaborator

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

@@ -1,12 +1,11 @@
import type SourceMap from "./source-map";
import type * as t from "@babel/types";
import * as charcodes from "charcodes";

type Pos = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you are exporting Loc, you should make sure to export Pos to

@@ -185,7 +185,7 @@ export function NullLiteral(this: Printer) {
}

export function NumericLiteral(this: Printer, node: t.NumericLiteral) {
const raw = this.getPossibleRaw(node);
const raw = this.getPossibleRaw(node) as string | undefined;
Copy link
Contributor

@armano2 armano2 Jun 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: if we know that this is always going to be a string or undefined, why not change getPossibleRaw instead of doing casting everywhere?

in some cases you are casting it to string in some to undefined or string, and in other you are casting them when you are using it

this.token(raw as string);

i understand that extra can't be strongly typed for now, but maybe that is going to change in feature

@@ -89,11 +100,11 @@ export function WhileStatement(this: Printer, node: t.WhileStatement) {
this.printBlock(node);
}

const buildForXStatement = function (op) {
return function (this: Printer, node: any) {
const buildForXStatement = function (op: "in" | "of") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a small refactor, but can we remove this parameter and just check node.type === "ForOfStatement" ? "of" : "in"?

return function (this: Printer, node: any) {
this.word(prefix);
function printStatementAfterKeyword(
this: Printer,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can make the printer a normal parameter (instead of this) so that we don't need to .call to use this function.

@@ -202,7 +202,7 @@ export function NumericLiteral(this: Printer, node: t.NumericLiteral) {
export function StringLiteral(this: Printer, node: t.StringLiteral) {
const raw = this.getPossibleRaw(node);
if (!this.format.minified && raw != null) {
this.token(raw);
this.token(raw as string);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't the != null check above help TS with this? Maybe if we make the return type of getPossibleRaw string | undefined instead of string | void it works?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We haven't enabled strictNullChecks so string | undefined will be considered as string by TS currently, which renders raw as string useless even without the null guard. With that said, I agree that string | undefined is more proper than string | void.

@@ -347,7 +351,11 @@ export function TSIntersectionType(this: Printer, node: t.TSIntersectionType) {
this.tsPrintUnionOrIntersectionType(node, "&");
}

export function tsPrintUnionOrIntersectionType(this: Printer, node: any, sep) {
export function tsPrintUnionOrIntersectionType(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to export this, or tsPrintBraced

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, and there are many other internal printer methods, too. I will take care of them in another PR.

Comment on lines 58 to 84
export function needsWhitespace(node, parent, type) {
export function needsWhitespace(
node: t.Node,
parent: t.Node,
type: "before" | "after",
): boolean | 0 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make a small modification to make it return boolean?

JLHwung and others added 4 commits June 13, 2022 09:03
Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>
Co-authored-by: liuxingbaoyu <30521560+liuxingbaoyu@users.noreply.github.com>
@JLHwung JLHwung merged commit 9a69ece into babel:noImplicitAny Jun 13, 2022
@JLHwung JLHwung deleted the noImplicitAny-generator branch June 13, 2022 13:51
JLHwung added a commit that referenced this pull request Jun 20, 2022
* generator

* refactor: merge {start,end}Terminatorless to printTerminatorless

* inline buildYieldAwait

* inline ExportDeclaration

* also export Pos

* let getPossibleRaw return string | void

* Apply suggestions from code review

Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>
Co-authored-by: liuxingbaoyu <30521560+liuxingbaoyu@users.noreply.github.com>

* address review comments

* do not export internal printer method

* simplify needsWhitespace

Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>
Co-authored-by: liuxingbaoyu <30521560+liuxingbaoyu@users.noreply.github.com>
JLHwung added a commit that referenced this pull request Jun 21, 2022
* generator

* refactor: merge {start,end}Terminatorless to printTerminatorless

* inline buildYieldAwait

* inline ExportDeclaration

* also export Pos

* let getPossibleRaw return string | void

* Apply suggestions from code review

Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>
Co-authored-by: liuxingbaoyu <30521560+liuxingbaoyu@users.noreply.github.com>

* address review comments

* do not export internal printer method

* simplify needsWhitespace

Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>
Co-authored-by: liuxingbaoyu <30521560+liuxingbaoyu@users.noreply.github.com>
JLHwung added a commit that referenced this pull request Jun 21, 2022
* generator

* refactor: merge {start,end}Terminatorless to printTerminatorless

* inline buildYieldAwait

* inline ExportDeclaration

* also export Pos

* let getPossibleRaw return string | void

* Apply suggestions from code review

Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>
Co-authored-by: liuxingbaoyu <30521560+liuxingbaoyu@users.noreply.github.com>

* address review comments

* do not export internal printer method

* simplify needsWhitespace

Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>
Co-authored-by: liuxingbaoyu <30521560+liuxingbaoyu@users.noreply.github.com>
JLHwung added a commit that referenced this pull request Jun 21, 2022
* enable noImplicitAny

* codemod (#14602)

* Improve preset/plugin-typescript typings (#14603)

* preset-typescript

* transform-typescript

* Update packages/babel-plugin-transform-typescript/src/enum.ts

Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>

* prettier

Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>

* Improve transform-runtime typings (#14605)

* enable noImplicitAny

* codemod (#14602)

* Improve preset/plugin-typescript typings (#14603)

* preset-typescript

* transform-typescript

* Update packages/babel-plugin-transform-typescript/src/enum.ts

Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>

* prettier

Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>

* improve transform-runtime typings

Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>

* jsx (#14607)

* Improve module transform packages typings (#14608)

* improve babel-template typings (#14621)

* Improve @babel/helper-* typings (#14620)

* improve helper-function-name typings

* helper hoist variables

* helper-member-expression-to-functions

* helper-simple-access

* remap-async-to-generator

* helper-annotate-as-pure

* helper-builder-binary-assignment-operator-visitor

* helper-check-duplicate-nodes

* early return when export declaration is export all

* split-export-declaration

* helper-define-map

* define-map

* Update packages/babel-helper-builder-binary-assignment-operator-visitor/src/index.ts

Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>

* review comments

Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>

* improve helper-module-imports typings (#14623)

* refactor: simplify ImportInjector._applyDefaults

* helper-module-imports

* map globals to the one used in Babel 8

* Improve fixture-test-runner typings (#14625)

* plugin-test-runner

* fixture-test-runner

* Update packages/babel-helper-fixtures/src/index.ts

Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>

Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>

* Improve preset-env typings (#14606)

* refactor: move shipped-proposals to ts

* preset-env

* address review comments

* Improve pipeline-operator typings (#14629)

* refactor: simplify buildOptimizedSequenceExpression

* proposal-pipeline-operator

* address review comments

* improve standalone typings (#14630)

* babel-standalone

* address review comment

* Improve plugins typings (Part 1) (#14633)

* change AssumptionFunction return type

* helper-create-regexp-features-plugin

* create-class-features-plugin

* transform-for-of

* improve unicode-escapes typings

* transform-object-super

* transform-react-constant-elements

* proto-to-assign

* function-name

* flow-comments

TS cannot infer Flow visitor type because we have both Flow type and Flow virtual type.

* Improve code-frame/hightlight typings (#14643)

* code-frame

* map js-tokens to Babel 8

* highlight

* Improve `@babel/types` typings (#14645)

* bump to-fast-properties to v4

* types

* mark node comments as mutable

* Improve babel-core typings (#14622)

Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>

* improve transform-classes typings (#14624)

* Improve `@babel/generator` typings (#14644)

* generator

* refactor: merge {start,end}Terminatorless to printTerminatorless

* inline buildYieldAwait

* inline ExportDeclaration

* also export Pos

* let getPossibleRaw return string | void

* Apply suggestions from code review

Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>
Co-authored-by: liuxingbaoyu <30521560+liuxingbaoyu@users.noreply.github.com>

* address review comments

* do not export internal printer method

* simplify needsWhitespace

Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>
Co-authored-by: liuxingbaoyu <30521560+liuxingbaoyu@users.noreply.github.com>

* Improve plugins typings (Part 2) (#14639)

* exponentiation-operator

* duplicate-keys

* computed-properties

* replace-supers

* block-scoping

* block-scoped-functions

* syntax-typescript

* record-and-tuple

* private-property-in-object

* partial-application

* json-strings

* function-sent

* function-bind

* class-static-block

* async-generator-functions

* Update packages/babel-plugin-transform-block-scoping/src/index.ts

* address review comments

* helper-replace-supers

* Improve plugins typings (Part 3) (#14642)

* external-helpers

* bugfix

* transform-parameters

* object-rest-spread

* destructuring-private

* transform-destructuring

* proposal-decorators

* optional-chaining

* helper-wrap-function

* explode-assignable-expression

* helper-compilation-targets

* helper-plugin-utils

* helpers

* helper-validator-option

* fix: allow "+" and "-" in MappedType .readonly/.optional

* fixture-test-runner

* remove charcodes from dependencies

the charcodes transform will inline charCodes.* so we can remove it from dependencies

* Improve traverse typings (#14648)

* export removeProperties options

* traverse

* fix other packages typings

* fix gatherNodeParts

* downgrade to-fast-properties to v3

* fix typo

* babel-core/normalize-file

* add ignore comment property used by flow plugin

* refine getLastStatement typings

* fix babel-standalone rebase typing error

* fix assert.rejects polyfill

* simplify _param typings

* supress Babel 7 AST errors

* loosen defineType typings

The AST typings are generated from defineType calls, so defineType should accept a string as type.

* restore legacy code

* Suppress data/core-js-compat importing errors

Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>
Co-authored-by: liuxingbaoyu <30521560+liuxingbaoyu@users.noreply.github.com>
@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 Sep 13, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 13, 2022
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants