Skip to content

Commit

Permalink
Deprecate NativeModule.onCatalystInstanceDestroy()
Browse files Browse the repository at this point in the history
Summary:
## Rationale
The CatalystInstance is going away after we delete the bridge. So, we should migrate away from onCatalystInstanceDestroy() to something else: invalidate().

## Changes
- Introduce the NativeModule.invalidate() cleanup hook.
- Both the NativeModule and TurboModule infra now call this invalidate() method, **as opposed to** onCatalystInstanceDestroy(), to perform NativeModule cleanup.
- **Is this safe?** All our NativeModules extend BaseJavaModule. BaseJavaModule.invalidate() delegates to NativeModule.onCatalystInstanceDestroy(), so NativeModules that implement onCatalystInstanceDestroy(), but not invalidate(), still have their onCatalystInstanceDestroy() method called.

Changelog: [Android][Deprecated] - Deprecate NativeModule.onCatalystInstanceDestroy() for NativeModule.invalidate()

Reviewed By: JoshuaGross

Differential Revision: D26871001

fbshipit-source-id: e3bdfa0cf653ecbfe42791631bc6229af62f4817
  • Loading branch information
RSNara authored and facebook-github-bot committed Mar 7, 2021
1 parent d477f80 commit 18c8417
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -61,16 +61,18 @@ public boolean canOverrideExistingModule() {
}

@Override
public void onCatalystInstanceDestroy() {
// do nothing
}
public void onCatalystInstanceDestroy() {}

public boolean hasConstants() {
return false;
}

// Cleanup Logic for TurboModules
/**
* The CatalystInstance is going away with Venice. Therefore, the TurboModule infra introduces the
* invalidate() method to allow NativeModules to clean up after themselves.
*/
@Override
public void invalidate() {
// Do nothing
onCatalystInstanceDestroy();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ public boolean canOverrideExistingModule() {
}

@Override
public void onCatalystInstanceDestroy() {
public void onCatalystInstanceDestroy() {}

@Override
public void invalidate() {
mHybridData.resetNative();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public ModuleHolder(NativeModule nativeModule) {

public synchronized void destroy() {
if (mModule != null) {
mModule.onCatalystInstanceDestroy();
mModule.invalidate();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,13 @@ interface NativeMethod {
*/
boolean canOverrideExistingModule();

/** Called before {CatalystInstance#onHostDestroy} */
/**
* Allow NativeModule to clean up. Called before {CatalystInstance#onHostDestroy}
*
* @deprecated use {@link #invalidate()} instead.
*/
void onCatalystInstanceDestroy();

/** Allow NativeModule to clean up. Called before {CatalystInstance#onHostDestroy} */
void invalidate();
}
Original file line number Diff line number Diff line change
Expand Up @@ -326,8 +326,7 @@ public void onCatalystInstanceDestroy() {
final TurboModule turboModule = getModule(moduleName, moduleHolder, false);

if (turboModule != null) {
// TODO(T48014458): Rename this to invalidate()
((NativeModule) turboModule).onCatalystInstanceDestroy();
turboModule.invalidate();
}
}

Expand Down

0 comments on commit 18c8417

Please sign in to comment.