-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Update babylon beta 3 #5394
Update babylon beta 3 #5394
Conversation
@@ -12,8 +12,6 @@ export function RestElement(node: Object) { | |||
|
|||
export { | |||
RestElement as SpreadElement, | |||
RestElement as SpreadProperty, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing since it won't be generated anymore
@@ -0,0 +1 @@ | |||
z = { x, ...y }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't be there. removing
@@ -61,23 +61,3 @@ defineType("ExportNamespaceSpecifier", { | |||
} | |||
} | |||
}); | |||
|
|||
defineType("RestProperty", { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add these as virtual types in https://github.com/babel/babel/blob/7.0/packages/babel-traverse/src/path/lib/virtual-types.js, e.g.
export const RestProperty = {
types: ["RestElement"],
checkPath(path: NodePath): boolean {
return path.parentPath && path.parentPath.isObjectPattern();
},
};
export const SpreadProperty = {
types: ["RestElement"],
checkPath(path: NodePath): boolean {
return path.parentPath && path.parentPath.isObjectExpression();
},
};
it might make it easier for people to convert a plugin from 6.x to 7.x
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok we can do that, although the visitors are still gone since ast is changed, so would that do much?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If other people write plugins that visit these types, this would keep the visitors working.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to do that with the rest of the ast changes then?
ForAwaitStatement, NumericLiteralTypeAnnotation, ExistentialTypeParam
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And then do we want to console.warn or something for these?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It if is doable for a type, I'd vote we do it, but it won't work for all of them.
For .warn
I'm not sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You want to do this virtual types in 6.x? imho it doesn't make sense to introduce bc code in a major version.
2aa8d1a
to
122d983
Compare
Ok added types - looked at babel/website#1146 for ast node changes |
* Update babylon to v7-beta.3 * convert RestProperty/SpreadProperty to RestElement/SpreadElement * add virtual types to make it easier to upgrade
The changes were either simple rename or code removal