Skip to content

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