Skip to content
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

fix: android duplicates in linked packages #81

Conversation

thymikee
Copy link
Member

@thymikee thymikee commented Jan 11, 2019

Fixes #43

  • Improved the Regex to be more forgiving in terms of spacing and implementation/compile names with variants and configs
  • add more thorough test
  • cleanup makeBuildPatch test to not test implementation details
  • cleanup link test so it doesn't output to stdout unnecessarily
  • tested on a project I had issues with duplicates (resolved with this diff)

cc @ferrannp

['test-compile-debug', true],
['test-compile-abc', true],
['test-not-there-yet', false],
])(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dope

@Esemesek Esemesek merged commit 4b1aa98 into react-native-community:master Jan 11, 2019
@grabbou
Copy link
Member

grabbou commented Jan 11, 2019

Awesome!

@matei-radu
Copy link
Contributor

matei-radu commented Mar 11, 2019

@grabbou thanks for making me aware of this PR!

These changes are solid but it doesn't seem to address the unlink issue. makeBuildPatch still returns an "implementation" patch string, so a previously linked "compile" dependency would not be successfully unlinked.

Let's say we want to unlink a "compile" dependency that was linked way back. When the revokePatch function is called in unregisterNativeModule.js#L41 it will look for an "implementation" string to replace with an empty string but, since we have a "compile" dependency, our dependency entry in the Gradle file will never be found and thus it would not be removed.

I can try to move my old PR from the RN repo to this one and continue the discussion there if that would be better 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix duplicate Android packages being linked
5 participants