Ensure correct Babel plugin locations in packager transform #4248
Conversation
|
By analyzing the blame information on this pull request, we identified @davidaurelio, @sahrens and @spicyj to be potential reviewers. |
facebook-github-bot
added
GH Review: review-needed
CLA Signed
| // using the plugins installed in the react-native package. | ||
| config.plugins = config.plugins.map(function(plugin) { | ||
| if (!Array.isArray(plugin)) plugin = [plugin]; | ||
| plugin[0] = require(`babel-plugin-${plugin[0]}`); |
There was a problem hiding this comment.
Mightn't plugin[0] already be the plugin object at this point?
There was a problem hiding this comment.
Yep... At least for one of them.
…mpt to resolve all base plugins relative to the file it's compiling. This makes sure that we're using the plugins installed in the react-native package.
|
Tests pass. |
|
Looks good to me FWIW. My only (small) concern is the code duplication with Babel's normalising logic -- maybe add a comment saying to keep this snippet logically in sync with Babel's behavior? Really don't feel strongly either way. Green tests are beautiful =) |
|
I had tried something similar locally but had the same concerns as @ide, but I'm okay with shipping it so we have green tests again, I think we can iterate later if necessary. |
|
@facebook-github-bot shipit |
|
Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/1078005122219412/int_phab to review. |
| // Normalise plugin to an array. | ||
| if (!Array.isArray(plugin)) { | ||
| plugin = [plugin]; | ||
| } |
There was a problem hiding this comment.
Just so I can get a better understanding of it: are non-arrays only supported for strings, i.e. module names to import?
Would this code have the same effect as
return typeof plugin === 'string' ? [require(`babel-plugin-${plugin}`)] : plugin;?
There was a problem hiding this comment.
Each entry in the plugins array can be a plugin or a [plugin, options] pair. In either case, the plugin can be the name of a plugin or the plugin itself.
|
Thanks for making us green again, @sebmck! |
|
Thanks so much @sebmck! 🎆 |
|
There are now a few new Flow errors that landed just before this but should be easy to fix, I'll take care of that and then master is hopefully green 👍 |
Summary: Manually resolve all default Babel plugins. `babel.transform` will attempt to resolve all base plugins relative to the file it's compiling. This makes sure that we're using the plugins installed in the react-native package. **NOTE:** Haven't tested this. Please don't merge until Travis has done CI. Closes facebook#4248 Reviewed By: svcscm Differential Revision: D2679651 Pulled By: davidaurelio fb-gh-sync-id: 0cdef1e738ba28c09c811432a71047f80540ed7b
Summary: Manually resolve all default Babel plugins. `babel.transform` will attempt to resolve all base plugins relative to the file it's compiling. This makes sure that we're using the plugins installed in the react-native package. **NOTE:** Haven't tested this. Please don't merge until Travis has done CI. Closes facebook#4248 Reviewed By: svcscm Differential Revision: D2679651 Pulled By: davidaurelio fb-gh-sync-id: 0cdef1e738ba28c09c811432a71047f80540ed7b
Summary: Manually resolve all default Babel plugins. `babel.transform` will attempt to resolve all base plugins relative to the file it's compiling. This makes sure that we're using the plugins installed in the react-native package. **NOTE:** Haven't tested this. Please don't merge until Travis has done CI. Closes facebook/react-native#4248 Reviewed By: svcscm Differential Revision: D2679651 Pulled By: davidaurelio fb-gh-sync-id: 0cdef1e738ba28c09c811432a71047f80540ed7b
Manually resolve all default Babel plugins.
babel.transformwill attempt to resolve all base plugins relative to the file it's compiling. This makes sure that we're using the plugins installed in the react-native package.NOTE: Haven't tested this. Please don't merge until Travis has done CI.