-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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 shared Promise.resolve() babel transform #27960
Add shared Promise.resolve() babel transform #27960
Conversation
Hey @erwinmombay, these files were changed:
Hey @gmajoulet, these files were changed:
Hey @newmuis, these files were changed:
Hey @Enriqe, these files were changed:
|
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 know Babel processing is mainly about transforming files, but does it also have the ability to create new ones? For this transform in particular, it would be neat if it could generate the src/resolved-promise.js
file.
All my review comments were nits/questions for my own learning.
LGTM 🚢
module.exports = function (babel, options = {}) { | ||
const {types: t} = babel; | ||
const promiseResolveMatcher = t.buildMatchMemberExpression('Promise.resolve'); | ||
const {importFrom = 'src/resolved-promise'} = options; |
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.
Another way to solve the "transform recursion" issue would be if the transform early-exited when transforming the importFrom
module.
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.
Yah.
@@ -0,0 +1,52 @@ | |||
/** | |||
* Copyright 2018 The AMP HTML Authors. All Rights Reserved. |
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.
nit: 2020.
Q: does having the correct current year matter? i.e. is this worth the time spent commenting/fixing?
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.
Not really. I end up copy pasting this all the time, and forgetting to update.
* Add shared Promise.resolve() babel transform * Fixes * Add dependency allowList exceptions * Fix types
Re: #27901