Skip to content

Enforce type checking on babel-{cli,node}#14765

Merged
liuxingbaoyu merged 9 commits intobabel:mainfrom
liuxingbaoyu:ts-cli-node
Jul 20, 2022
Merged

Enforce type checking on babel-{cli,node}#14765
liuxingbaoyu merged 9 commits intobabel:mainfrom
liuxingbaoyu:ts-cli-node

Conversation

@liuxingbaoyu
Copy link
Copy Markdown
Member

@liuxingbaoyu liuxingbaoyu commented Jul 18, 2022

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

These two packages seem to be left out.

I use a couple of @ts-expect-error because the imported packages have no or incorrect type definitions, but that doesn't matter because only one method belonging to them is used.

@liuxingbaoyu liuxingbaoyu added PR: Internal 🏠 A type of pull request used for our changelog categories pkg: cli pkg: node area: typescript labels Jul 18, 2022
Comment thread packages/babel-cli/src/babel/dir.ts
Comment thread packages/babel-cli/src/babel/file.ts Outdated
@@ -1,5 +1,7 @@
import convertSourceMap from "convert-source-map";
import { AnyMap, encodedMap } from "@jridgewell/trace-mapping";
import type { Section } from "@jridgewell/trace-mapping/dist/types/types";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am not comfortable with the undeclared package sub imports. @jridgewell Can you make the Section type public?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sure. Does this serve the need for now?

import type { SectionedSourceMap } from '@jridgewell/trace-mapping';
type Section = SectionedSourceMap['sections'][0]

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, Thanks!

Comment thread packages/babel-cli/src/babel/watcher.ts Outdated
pollInterval: 10,
},
});
} as WatchOptions);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does const options: WatchOptions = { ... } work? Casting WatchOptions will mute any chokidar config errors.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes it can, but it should be able to type check even with as WatchOptions.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

IIUC using as is basically a cast, it will fail if the types are incompatible but it won't point out things like missing required properties.

@JLHwung JLHwung changed the title fix: Apply options of typescript to cli and node Enforce type checking on babel-{cli,node} Jul 18, 2022
Comment thread packages/babel-node/src/babel-node.ts Outdated
Co-authored-by: Huáng Jùnliàng <jlhwung@gmail.com>
@babel-bot
Copy link
Copy Markdown
Collaborator

babel-bot commented Jul 18, 2022

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

Comment thread lib/slash.d.ts Outdated
Copy link
Copy Markdown
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

Approving but I would like to see #14765 (comment) resolved

filter?: (filename: string, index: number, dir: string) => boolean
): string[];
export = read;
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As a follow-up, could you open a PR to update https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/fs-readdir-recursive/index.d.ts so that then we can use @types/fs-readdir-recursive?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

DefinitelyTyped/DefinitelyTyped#61344
Finish. I think we might be able to remove this package at some point.

Comment thread packages/babel-node/src/_babel-node.ts Outdated
liuxingbaoyu and others added 2 commits July 20, 2022 11:13
Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>
Comment thread packages/babel-cli/src/babel/file.ts Outdated
Comment thread packages/babel-cli/src/babel/file.ts Outdated
liuxingbaoyu and others added 2 commits July 20, 2022 11:32
Co-authored-by: Justin Ridgewell <justin@ridgewell.name>
@liuxingbaoyu liuxingbaoyu merged commit b807206 into babel:main Jul 20, 2022
@liuxingbaoyu liuxingbaoyu deleted the ts-cli-node branch July 20, 2022 04:50
@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 Oct 20, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area: typescript outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: cli pkg: node 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.

6 participants