Skip to content

fix: monkey patched node was cloned#913

Merged
AndyOGo merged 1 commit intodevelopfrom
bugfix/monkey-patch
Apr 12, 2019
Merged

fix: monkey patched node was cloned#913
AndyOGo merged 1 commit intodevelopfrom
bugfix/monkey-patch

Conversation

@AndyOGo
Copy link

@AndyOGo AndyOGo commented Apr 12, 2019 •
edited
Loading

Fixes #778
Should also handle #859

Changes proposed in this pull request:

  • Fixes monkey patching breaks react refs by cloning new nodes

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@AndyOGo AndyOGo self-assigned this Apr 12, 2019
@AndyOGo
Copy link
Author

AndyOGo commented Apr 12, 2019

fix-react-dynamic-children

@raphaellueckl raphaellueckl removed their request for review April 12, 2019 08:48
LucaMele
LucaMele approved these changes Apr 12, 2019
View reviewed changes
Copy link
Contributor

@LucaMele LucaMele left a comment

Choose a reason for hiding this comment

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

LGTM. @AndyOGo i wont have time to test so please only merge if u can fix related bugs after merging

@AndyOGo AndyOGo merged commit d3c7ecf into develop Apr 12, 2019
@AndyOGo AndyOGo deleted the bugfix/monkey-patch branch April 12, 2019 14:43
@AndyOGo AndyOGo mentioned this pull request Apr 15, 2019
Closed
@AndyOGo AndyOGo mentioned this pull request Apr 30, 2019
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@davidknezic davidknezic Awaiting requested review from davidknezic

@jackblackCH jackblackCH Awaiting requested review from jackblackCH

@jan-capiak jan-capiak Awaiting requested review from jan-capiak

@MarekLacoAXA MarekLacoAXA Awaiting requested review from MarekLacoAXA

@markus-walther markus-walther Awaiting requested review from markus-walther

@MKaHead MKaHead Awaiting requested review from MKaHead

@nakovskijosif nakovskijosif Awaiting requested review from nakovskijosif

@01231 01231 Awaiting requested review from 01231

@TheDadi TheDadi Awaiting requested review from TheDadi

1 more reviewer

@LucaMele LucaMele LucaMele approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@AndyOGo AndyOGo

Labels

Bug High-prio PROD_ISSUE

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

dynamic custom elements loose react refs, never connect and do not update props

2 participants

@AndyOGo @LucaMele