Fix Mangle parameters not taken into account #749
Conversation
|
Hi @mfbx9da4! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
mfbx9da4
changed the title
mfbx9da4
changed the title
facebook-github-bot
added
the
CLA Signed
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
facebook-github-bot
added
the
Shared with Meta
|
@rh389 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
| mangle: | ||
| typeof config.mangle === 'object' | ||
| ? { | ||
| ...config.mangle, | ||
| reserved, | ||
| } | ||
| : config.mangle, | ||
| sourceMap: map | ||
| ? { | ||
| ...config.sourceMap, | ||
| content: map, | ||
| } | ||
| ? typeof config.sourceMap === 'object' | ||
| ? { | ||
| ...config.sourceMap, | ||
| content: map, | ||
| } | ||
| : config.sourceMap |
There was a problem hiding this comment.
Hi @mfbx9da4, thanks for looking into this.
I think we need just to be a bit careful around intention here - if user config is null or undefined we should respect metro's defaults, and continue to pass an object downstream.
IMO the best approach would be to check whether the user explicitly wants to disable these features with === false, and spread everything else (note that it is safe to object-spread primitives)
There was a problem hiding this comment.
Yeah great point. Updated now, let me know if that works! 🙂
|
@rh389 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
Glad to see it's been approved. Would you like me to squash my commits or will you do that when you merge via github's |
|
Hi @mfbx9da4 - the process is a little bit idiosyncratic because the source of truth here is another repo within Meta, mirrored to GitHub. I fixed up that whitespace internally and the system is going through the process of landing the (squashed) commit now - a bot will let us know here when it does. |
…ebook#749) Summary: Fixes facebook#301 Pull Request resolved: facebook#749 Reviewed By: motiz88 Differential Revision: D33398597 Pulled By: rh389 fbshipit-source-id: c6fff45e20df500977215bebe6707d0bb84e84d2
…ebook#749) Summary: Fixes facebook#301 Pull Request resolved: facebook#749 Reviewed By: motiz88 Differential Revision: D33398597 Pulled By: rh389 fbshipit-source-id: c6fff45e20df500977215bebe6707d0bb84e84d2
…ebook#749) Summary: Fixes facebook#301 Pull Request resolved: facebook#749 Reviewed By: motiz88 Differential Revision: D33398597 Pulled By: rh389 fbshipit-source-id: c6fff45e20df500977215bebe6707d0bb84e84d2
Fixes #301