-
Notifications
You must be signed in to change notification settings - Fork 28
Babel preset: Enable support for async generator functions #126
Conversation
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.
LGTM 👍
@@ -0,0 +1,15 @@ | |||
describe( 'Babel preset default', () => { |
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.
Depending on the environment in which these tests are run (i.e. Node v10+ where asynchronous generators are supported natively), couldn't these tests pass even if the transform were not applied?
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.
Aren't we using LTS (node 8) on Travis?
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.
Only until October: https://github.com/nodejs/Release
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.
They are much faster this time 👍
We can simply remove this test if it isn't useful in the long run. What would you recommend instead?
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.
I don't know that I would test a preset, which is effectively intended as a configuration. Maybe a snapshot of transpiled code? That the configuration includes expected plugins?
Async generator functions were enabled in Gutenberg to use with
data
module. See:https://github.com/WordPress/gutenberg/blob/master/package.json#L108
Given that plugins might want to use the same syntax, let's enable this Babel plugin for everyone.