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-core typings #14622

Merged
merged 14 commits into from Jun 11, 2022

Conversation

liuxingbaoyu
Copy link
Member

@liuxingbaoyu liuxingbaoyu commented May 30, 2022

Q                       A
License MIT

See #14601 for more info.

@babel-bot
Copy link
Collaborator

babel-bot commented May 30, 2022

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

@liuxingbaoyu
Copy link
Member Author

I didn't modify packages\babel-core\src\transformation\util\clone-deep.ts because of #14583.

sourceType: "string" | "module";
sourceType: "script" | "module";
Copy link
Member Author

Choose a reason for hiding this comment

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

Seems to be a bug.

packages/babel-core/src/config/caching.ts Outdated Show resolved Hide resolved
packages/babel-core/src/config/config-chain.ts Outdated Show resolved Hide resolved
@@ -72,7 +72,7 @@ export function* resolveShowConfigPath(
return null;
}

export const ROOT_CONFIG_FILENAMES = [];
export const ROOT_CONFIG_FILENAMES: any[] = [];
Copy link
Member

Choose a reason for hiding this comment

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

This is probably an array of strings?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know, it's an empty implementation file. But string[] should be fine.

packages/babel-core/src/config/files/module-types.ts Outdated Show resolved Hide resolved
function enhanceError<T extends Function>(context, fn: T): T {
return function* (arg1, arg2) {
function enhanceError<T extends Function>(context: any, fn: T): T {
return function* (arg1: unknown, arg2: unknown) {
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 use Parameters<T>[0] and Parameters<T>[1]

Copy link
Member Author

Choose a reason for hiding this comment

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

It doesn't seem to be, it can't be passed into the Function.

Copy link
Contributor

@armano2 armano2 Jun 6, 2022

Choose a reason for hiding this comment

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

you should be able to use Parameters in here if you define proper correct type of T

i'm not sure what should be in here, but as simple example you can do something like this:

function enhanceError<T extends (arg1?: unknown, arg2?: unknown) => Generator<unknown>>(
  context: ConfigContext,
  fn: T,
): T {
  return function* (arg1: Parameters<T>[0], arg2: Parameters<T>[1]) {

see https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-6.html

packages/babel-core/src/config/full.ts Outdated Show resolved Hide resolved
@@ -490,11 +490,11 @@ const instantiatePreset = makeWeakCacheSync(
},
);

function chain(a, b) {
function chain(a: any, b: any) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
function chain(a: any, b: any) {
function chain<Args extends any[])(a: undefined | (...args: Args) => void, b: undefined | (...args: Args) => void) {

and use Args instead of unknown[] below.

Copy link
Member Author

Choose a reason for hiding this comment

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

export default class Plugin {
  key: string | undefined | null;
  manipulateOptions?: (options: unknown, parserOpts: unknown) => void;
  post?: Function;
  pre?: Function;
  visitor: {};

Pre and post are passed in, and the types do not match. Do I need to modify the Plugin type?

packages/babel-core/src/config/helpers/config-api.ts Outdated Show resolved Hide resolved
packages/babel-core/src/config/helpers/config-api.ts Outdated Show resolved Hide resolved
@@ -1,4 +1,5 @@
import gensync from "gensync";
Copy link
Member

Choose a reason for hiding this comment

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

Todo (for myself): Continue reviewing from this file.

Copy link
Contributor

@JLHwung JLHwung left a comment

Choose a reason for hiding this comment

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

Awesome work.

I left some comments about Function. I think it should only be used when we have no idea about its signature, otherwise we'd better explicitly specify its input and output.

packages/babel-core/src/config/config-chain.ts Outdated Show resolved Hide resolved
packages/babel-core/src/config/config-chain.ts Outdated Show resolved Hide resolved
packages/babel-core/src/config/files/types.ts Outdated Show resolved Hide resolved
packages/babel-core/src/config/full.ts Outdated Show resolved Hide resolved
@@ -1,4 +1,7 @@
const pluginNameMap = {
const pluginNameMap: Record<
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we mark this object as const? So we don't have to add typings at all and TS will infer the typings accordingly.

Copy link
Member Author

Choose a reason for hiding this comment

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

It doesn't seem to work, the object will be written below.

packages/babel-core/src/tools/build-external-helpers.ts Outdated Show resolved Hide resolved
packages/babel-core/src/transform-file-browser.ts Outdated Show resolved Hide resolved
packages/babel-core/src/config/index.ts Outdated Show resolved Hide resolved
) {
if (typeof opts === "function") {
callback = opts;
opts = undefined;
opts = undefined as ValidatedOptions;
Copy link
Contributor

Choose a reason for hiding this comment

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

Q: why we need to cast undefined as ValidatedOptions?

Copy link
Member Author

Choose a reason for hiding this comment

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

Let the type of opts be ValidatedOptions from the function.

Copy link
Member

Choose a reason for hiding this comment

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

Isn't the opts parameter typed as InputOptions | undefined | null, so undefined should work? 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

opts: ValidatedOptions | FileParseCallback
Both ValidatedOptions and FileParseCallback can be undefined.

code: string = "";
inputMap: any | null = null;
inputMap: any;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it SourceMapConverter from "convert-source-map"?

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems yes, but there are such definitions in many files.
type SourceMap = any;
I don't know what the reason is.

packages\babel-core\src\transformation\file\generate.ts
packages\babel-core\src\transformation\index.ts
packages\babel-core\src\transformation\file\merge-map.ts

Copy link
Contributor

Choose a reason for hiding this comment

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

type SourceMap = any;

I believe it's a workaround due to lack of typings when we start transitioning to TS.

path: NodePath<t.Program> = null;
ast: any = {};
_map: Map<unknown, unknown> = new Map();
opts: { [key: string]: any };
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it ValidatedOptions from packages/babel-core/src/config/validation/options.ts returned by packages/babel-core/src/transformation/normalize-file.ts? Or should we come up with a new type NormalizedOptions?

Copy link
Member Author

Choose a reason for hiding this comment

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

Since I'm not familiar enough with babel internals, I can't confirm its type.
Maybe you can take a look after the PR is merged, I didn't change some code that was explicitly defined as any.

JLHwung
JLHwung previously approved these changes Jun 7, 2022
@JLHwung JLHwung dismissed their stale review June 8, 2022 13:28

CI error is related.

// NOTE: We want to return "null" explicitly, while ?. alone returns undefined
return config?.options ?? null;
});
const loadOptionsRunner = gensync<(opts: unknown) => ResolvedConfig | null>(
Copy link
Member

Choose a reason for hiding this comment

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

Future note: we will be able to drop our type definitions for gensync and use DefinitelyTyped/DefinitelyTyped#60125

packages/babel-core/src/config/resolve-targets-browser.ts Outdated Show resolved Hide resolved
packages/babel-core/src/config/util.ts Outdated Show resolved Hide resolved
packages/babel-core/src/gensync-utils/async.ts Outdated Show resolved Hide resolved
) {
if (typeof opts === "function") {
callback = opts;
opts = undefined;
opts = undefined as ValidatedOptions;
Copy link
Member

Choose a reason for hiding this comment

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

Isn't the opts parameter typed as InputOptions | undefined | null, so undefined should work? 🤔

if (typeof opts === "function") {
callback = opts;
opts = undefined;
opts = undefined as InputOptions;
Copy link
Member

Choose a reason for hiding this comment

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

(also here)

@nicolo-ribaudo
Copy link
Member

The linting error looks like a TS bug, I wonder if it's fixed in 4.7.

@nicolo-ribaudo nicolo-ribaudo merged commit ac8bb49 into babel:noImplicitAny Jun 11, 2022
JLHwung pushed a commit that referenced this pull request Jun 20, 2022
Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>
JLHwung pushed a commit that referenced this pull request Jun 21, 2022
Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>
JLHwung pushed a commit that referenced this pull request Jun 21, 2022
Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.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 11, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 11, 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