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 all missing babel-core option properties and alphabetize for better maintainability. #19428

Merged
merged 5 commits into from
Sep 7, 2017
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions types/babel-core/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@ export { t as types };
export type Node = t.Node;
export import template = require('babel-template');
export const version: string;
import traverse, { Visitor } from "babel-traverse";
import traverse, { Visitor, NodePath } from "babel-traverse";
Copy link
Contributor

Choose a reason for hiding this comment

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

This NodePath isn't used. Did you want to use it for wrapPluginVisitorMethod?

The rest looks great.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Left over from my trying to get wrapPluginVisitorMethod to have better specified types – was going to ask about that actually. The callback function it takes and the function it returns can take any NodePath variant. I couldn't come up with a way to add concrete typing to those functions without enumerating every templated type, like is done in babel-traverse. Do you have a better way to do this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Simple NodePath (AKA NodePath<Node>) is okay in this case.

export { traverse, Visitor };
import { BabylonOptions } from "babylon";
export { BabylonOptions };
import { GeneratorOptions } from "babel-generator";
export { GeneratorOptions };

// A babel plugin is a simple function which must return an object matching
// the following interface. Babel will throw if it finds unknown properties.
Expand All @@ -40,7 +42,6 @@ export function transformFileSync(filename: string, opts?: TransformOptions): Ba
export function transformFromAst(ast: Node, code?: string, opts?: TransformOptions): BabelFileResult;

export interface TransformOptions {

/** Include the AST in the returned object. Default: `true`. */
ast?: boolean;

Expand Down Expand Up @@ -164,8 +165,7 @@ export interface TransformOptions {
/** An optional callback that can be used to wrap visitor methods.
* NOTE: This is useful for things like introspection, and not really needed for implementing anything.
*/
wrapPluginVisitorMethod?(pluginAlias: string, visitorType: string, callback: Function): Function;

wrapPluginVisitorMethod?(pluginAlias: string, visitorType: string, callback: (path: any, state: any) => void): (path: any, state: any) => void ;
Copy link
Contributor

Choose a reason for hiding this comment

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

trying to get wrapPluginVisitorMethod to have better specified types – was going to ask about that actually. The callback function it takes and the function it returns can take any NodePath variant.

That's why the parent type should be used - NodePath (same as NodePath<Node>). All the variants are assignable to it, and there is no possibility to infer the specific subtype from anywhere, so the user has to use type guards or assertions anyway to work with specific subtypes. Which is unlikely however because the wrapPluginVisitorMethod option is intended for simpler use cases, see the original PR babel/babel#3659.

Copy link
Contributor

Choose a reason for hiding this comment

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

Since we're on it, visitorType can be typed as 'enter' | 'exit', can't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated 👍

}

export interface BabelFileResult {
Expand Down