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: Add missing React 18 reconciler function #206

Merged
merged 3 commits into from
Feb 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/four-rivers-destroy.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@remote-ui/react': patch
---

Added missing `detachDeletedInstance` function to `@remote-ui/react/reconciler`. This function is invoked during React's clean up phase, so prior to this change you'd get an exception / broken app when a component is removed from the tree.
4 changes: 4 additions & 0 deletions packages/react/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@

- [#191](https://github.com/Shopify/remote-ui/pull/191) [`77ba3da`](https://github.com/Shopify/remote-ui/commit/77ba3da217cb0e443e674afc058916ca25f5900e) Thanks [@lemonmade](https://github.com/lemonmade)! - Removed re-export of `@remote-ui/rpc`. If you need `retain` or `release`, import them directly from `@remote-ui/rpc` instead.

### Patch Changes

- Added missing `detachDeletedInstance` function to `@remote-ui/react/reconciler`. This function is invoked during React's clean up phase, so prior to this change you'd get an exception / broken app when a component is removed from the tree.

## 4.6.0

### Minor Changes
Expand Down
1 change: 1 addition & 0 deletions packages/react/src/reconciler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ export const createReconciler = (options?: {primary?: boolean}) =>
resetAfterCommit() {},
commitMount() {},
preparePortalMount() {},
detachDeletedInstance() {},
});

function scheduleMicrotask(callback: () => void) {
Expand Down
46 changes: 46 additions & 0 deletions packages/react/src/tests/e2e.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -856,4 +856,50 @@ describe('@remote-ui/react', () => {

expect(appElement.innerHTML).toContain('bac');
});

it('handles removal & cleanup of unmounted components', () => {
const receiver = createRemoteReceiver();
const remoteRoot = createRemoteRoot(receiver.receive, {
components: [RemoteImage, RemoteButton],
});

function RemoteApp() {
const [showImage, setShowImage] = useState(true);

return (
<>
{showImage && <RemoteImage data-test-id="image" />}
<RemoteButton onPress={() => setShowImage((prev) => !prev)}>
Toggle image
</RemoteButton>
</>
);
}

const controller = createController({
Button: HostButton,
Image: HostImage,
});

function HostApp() {
return <RemoteRenderer controller={controller} receiver={receiver} />;
}

domAct(() => {
domRoot.render(<HostApp />);
createRoot(remoteRoot).render(<RemoteApp />);
remoteRoot.mount();
jest.runAllTimers();
});

domAct(() => {
Simulate.click(appElement.querySelector('button')!);
});

domAct(() => {
jest.runAllTimers();
});

expect(appElement.innerHTML).not.toContain('data-test-id="image"');
});
});