Skip to content

[0.63] Fix native modules not being re-initialized on reload (#6159) - #6176

Merged
1 commit merged into
microsoft:0.63-stablefrom
tido64:backport-native-module-reinit-fix
Oct 7, 2020
Merged

[0.63] Fix native modules not being re-initialized on reload (#6159)#6176
1 commit merged into
microsoft:0.63-stablefrom
tido64:backport-native-module-reinit-fix

Conversation

@tido64

@tido64 Tommy Nguyen (tido64) commented Oct 2, 2020 •
edited by ghost
Loading

Copy link
Copy Markdown
Member

When calling ReactNativeHost::ReloadInstance(), ReactPackageProviders are not reiterated and thus not calling
AddAttributedModules(). As a consequence, the native modules that were loaded the first time the app was loaded, no longer "exist" after a reload.

Microsoft Reviewers: Open in CodeFlow

@tido64
Tommy Nguyen (tido64) requested a review from a team as a code owner October 2, 2020 21:05
@tido64

Tommy Nguyen (tido64) commented Oct 2, 2020

Copy link
Copy Markdown
Member Author

cc Vladimir Morozov (@vmoroz), acoates-ms

@tido64
Tommy Nguyen (tido64) force-pushed the backport-native-module-reinit-fix branch 2 times, most recently from 9a08fd3 to 811a5d6 Compare October 2, 2020 21:14
@dannyvv

Danny van Velzen (dannyvv) commented Oct 5, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

Looks like this code is already in master and this is the port to 0.63. Adding the appropriate tag for triage.

@acoates-ms acoates-ms changed the title Fix native modules not being re-initialized on reload (#6159) [0.63] Fix native modules not being re-initialized on reload (#6159) Oct 5, 2020
@tido64
Tommy Nguyen (tido64) force-pushed the backport-native-module-reinit-fix branch from 586fa80 to 073f378 Compare October 6, 2020 18:29
@tido64
Fix native modules not being re-initialized on reload (microsoft#6159)
e7b9341 
When calling `ReactNativeHost::ReloadInstance()`,
`ReactPackageProvider`s are not reiterated and thus not calling
`AddAttributedModules()`. As a consequence, the native modules that were
loaded the first time the app was loaded, no longer "exist" after
a reload.
@tido64
Tommy Nguyen (tido64) force-pushed the backport-native-module-reinit-fix branch from 073f378 to e7b9341 Compare October 7, 2020 15:35
@tido64

Tommy Nguyen (tido64) commented Oct 7, 2020

Copy link
Copy Markdown
Member Author

acoates-ms, Nick Gerleman (@NickGerleman), Vladimir Morozov (@vmoroz): Could you please take another look? Finally got all the checks to pass.

@NickGerleman

Nick Gerleman (NickGerleman) commented Oct 7, 2020

Copy link
Copy Markdown
Contributor

acoates-ms you were of the opinion this one was severe enough to be backported, right?

Chris Glein (@chrisglein) Alexander Sklar (@asklar) kmelmon Khalef Hosany (@Khalef1) any concerns backporting this?

@Khalef1

Khalef Hosany (Khalef1) commented Oct 7, 2020

Copy link
Copy Markdown
Contributor

acoates-ms you were of the opinion this one was severe enough to be backported, right?

Chris Glein (@chrisglein) Alexander Sklar (@asklar) kmelmon Khalef Hosany (@Khalef1) any concerns backporting this?

No concerns on my side for backporting

@acoates-ms

acoates-ms commented Oct 7, 2020

Copy link
Copy Markdown
Contributor

Yes we should take this.

@NickGerleman Nick Gerleman (NickGerleman) added the AutoMerge Causes a PR to be automatically merged once all requirements are passed (label drives bot activity) label Oct 7, 2020
@ghost

Deleted user (ghost) commented Oct 7, 2020

Copy link
Copy Markdown

Hello Nick Gerleman (@NickGerleman)!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost
ghost merged commit cefc50b into microsoft:0.63-stable Oct 7, 2020
@tido64
Tommy Nguyen (tido64) deleted the backport-native-module-reinit-fix branch October 7, 2020 20:41
@tido64 Tommy Nguyen (tido64) mentioned this pull request Oct 17, 2021
Closed
32 of 34 tasks
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers
@NickGerleman Nick Gerleman (NickGerleman) NickGerleman approved these changes
@jonthysell Jon Thysell (jonthysell) jonthysell approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

AutoMerge Causes a PR to be automatically merged once all requirements are passed (label drives bot activity)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@tido64 @dannyvv @NickGerleman @Khalef1 @acoates-ms @jonthysell @vmoroz