Add all missing `babel-core` option properties and alphabetize for better maintainability. #19428

amiller-gh · 2017-08-29T17:28:13Z

Please fill in this template.

Use a meaningful title for the pull request. Include the name of the package modified.
Test the change in your own code. (Compile and run.)
Follow the advice from the readme.
Avoid common mistakes.
Run npm run lint package-name (or tsc if no tslint.json is present).

Select one of these and delete the others:

If changing an existing definition:

Provide a URL to documentation or source code which provides context for the suggested changes: https://babeljs.io/docs/usage/api/#options
Increase the version number in the header if appropriate.
If you are making substantial changes, consider adding a tslint.json containing { "extends": "dtslint/dt.json" }.

dt-bot · 2017-08-29T17:28:15Z

types/babel-core/index.d.ts

to authors (@yortus @marvinhagemeister). Could you review this PR?
👍 or 👎?

Checklist

pass the Travis CI test?

typescript-bot · 2017-08-29T18:29:49Z

@amiller-gh Please fix the failures indicated in the Travis CI log.

thorn0 · 2017-08-29T18:52:22Z

Using types Object and Function is almost never a good idea. It's almost always (approx. in 99.99% of cases) possible to specify a better type.

amiller-gh · 2017-09-01T20:44:33Z

Will update – saw it used elsewhere in the file for Babylon config passthroughs, thought it may be okay.

thorn0 · 2017-09-01T21:15:38Z

You must have seen object, not Object.

Why not import BabylonOptions from the definitions for Babylon?

…properties alphabetically for maintainability.

amiller-gh · 2017-09-04T00:00:42Z

PR updated with a much less lazy change – Now all missing Babel Core options have been added, and all the options have been alphabetized like in the Babel documentation.

thorn0 · 2017-09-04T08:38:11Z

types/babel-core/index.d.ts

@@ -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";


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

The rest looks great.

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?

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

typescript-bot · 2017-09-04T21:49:22Z

@thorn0 - Thanks for your review of this PR! Can you please look at the new code and update your review status if appropriate?

thorn0 · 2017-09-05T09:04:26Z

types/babel-core/index.d.ts

@@ -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";


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

thorn0 · 2017-09-05T09:17:23Z

types/babel-core/index.d.ts

-
-    /** Specify a custom name for module ids. */
-    moduleId?: string;
+    wrapPluginVisitorMethod?(pluginAlias: string, visitorType: string, callback: (path: any, state: any) => void): (path: any, state: any) => void ;


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.

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

Updated 👍

thorn0 · 2017-09-06T10:36:35Z

Looks good. 👍 Formally, I'm not a reviewer of this package, so now I think you need an approve from one of them.

marvinhagemeister

LGTM 👍

typescript-bot · 2017-09-07T17:50:19Z

Approved by a listed owner. PR ready to merge pending express review by a maintainer.

* Enable new State / Substate method syntax - Bump css-blocks so correlations aren't enumerated. Update tests accordingly. - Remove explicit state import from blocks - States are now methods on Blocks and Classes - Static and dynamic substates may be passed to State methods - Add error checking for mis-use of boolean states - States with sub-states must be passed a value - Initial infrastructure added to construct boolean expressions - Use new css-block analysis introspection methods in tests - Move BlockObject lookup logic to ExpressionReader - Remove detailed ast and source data from Template when no longer needed. - Add new babel-core types file locally until DefinitelyTyped/DefinitelyTyped#19428 lands

Add parserOpts to TransformOptions interface for @types/babel-core

1b92e6a

typescript-bot added The Travis CI build failed Revision needed This PR needs code changes before it can be merged. labels Aug 29, 2017

Add all missing properties to interface for @types/babel-core. Order …

721f420

…properties alphabetically for maintainability.

Remove use of generic type, make tslint happy.

012dc2a

typescript-bot removed The Travis CI build failed Revision needed This PR needs code changes before it can be merged. labels Sep 4, 2017

thorn0 reviewed Sep 4, 2017

View reviewed changes

Remove stray NodePath import

061825f

amiller-gh changed the title ~~Add parserOpts to TransformOptions interface for @types/babel-core~~ Add all missing babel-core option properties and alphabetize for better maintainability. Sep 4, 2017

thorn0 suggested changes Sep 5, 2017

View reviewed changes

thorn0 reviewed Sep 5, 2017

View reviewed changes

Add improved types for wrapPluginVisitorMethod

a2f49b8

thorn0 approved these changes Sep 6, 2017

View reviewed changes

marvinhagemeister approved these changes Sep 6, 2017

View reviewed changes

typescript-bot added this to Merge: Express in Pull Request Triage Backlog Sep 7, 2017

typescript-bot added the Author Approved label Sep 7, 2017

typescript-bot added Other Approved This PR was reviewed and signed-off by a community member. Popular package This PR affects a popular package (as counted by NPM download counts). labels Sep 7, 2017

sandersn merged commit 7a3b23d into DefinitelyTyped:master Sep 7, 2017

sandersn removed this from Merge: Express in Pull Request Triage Backlog Sep 7, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add all missing `babel-core` option properties and alphabetize for better maintainability. #19428

Add all missing `babel-core` option properties and alphabetize for better maintainability. #19428

amiller-gh commented Aug 29, 2017

dt-bot commented Aug 29, 2017

typescript-bot commented Aug 29, 2017

thorn0 commented Aug 29, 2017

amiller-gh commented Sep 1, 2017 •

edited

thorn0 commented Sep 1, 2017

amiller-gh commented Sep 4, 2017 •

edited

thorn0 Sep 4, 2017

amiller-gh Sep 4, 2017

thorn0 Sep 5, 2017

typescript-bot commented Sep 4, 2017

thorn0 Sep 5, 2017

thorn0 Sep 5, 2017

thorn0 Sep 5, 2017

amiller-gh Sep 5, 2017

thorn0 commented Sep 6, 2017

marvinhagemeister left a comment

typescript-bot commented Sep 7, 2017

Add all missing babel-core option properties and alphabetize for better maintainability. #19428

Add all missing babel-core option properties and alphabetize for better maintainability. #19428

Conversation

amiller-gh commented Aug 29, 2017

dt-bot commented Aug 29, 2017

typescript-bot commented Aug 29, 2017

thorn0 commented Aug 29, 2017

amiller-gh commented Sep 1, 2017 • edited

thorn0 commented Sep 1, 2017

amiller-gh commented Sep 4, 2017 • edited

thorn0 Sep 4, 2017

Choose a reason for hiding this comment

amiller-gh Sep 4, 2017

Choose a reason for hiding this comment

thorn0 Sep 5, 2017

Choose a reason for hiding this comment

typescript-bot commented Sep 4, 2017

thorn0 Sep 5, 2017

Choose a reason for hiding this comment

thorn0 Sep 5, 2017

Choose a reason for hiding this comment

thorn0 Sep 5, 2017

Choose a reason for hiding this comment

amiller-gh Sep 5, 2017

Choose a reason for hiding this comment

thorn0 commented Sep 6, 2017

marvinhagemeister left a comment

Choose a reason for hiding this comment

typescript-bot commented Sep 7, 2017

Add all missing `babel-core` option properties and alphabetize for better maintainability. #19428

Add all missing `babel-core` option properties and alphabetize for better maintainability. #19428

amiller-gh commented Sep 1, 2017 •

edited

amiller-gh commented Sep 4, 2017 •

edited