toss / overlay-kit

The simplest and most intuitive way to manage overlays in React.
https://overlay-kit.slash.page
MIT License
296 stars 31 forks source link

Question: Intent on how overlay-kit Provider works #63

Closed XionWCFM closed 4 months ago

XionWCFM commented 4 months ago

Summery

The overlay kit provider seems to behave differently than other libraries. Is this intended behavior?

Detail

I realized that the following actions were needed in the test code to test the operation of overlay-kit.


afterEach(() => {
  overlay.unmountAll();
});

The reason this is necessary is because the OverlayProvider is separate from the elements mounted in the actual DOM.

Therefore, in most cases providing different OverlayProviders for individual tests does not make sense from the perspective of making the tests independent.

const wrapper = ({ children }: PropsWithChildren) => <OverlayProvider>{children}</OverlayProvider>;

However, libraries such as jotai can guarantee test independence by providing different providers.

import { Provider as JotaiProvider } from 'jotai'
const wrapper = ({ children }: PropsWithChildren) => <JotaiProvider>{children}</JotaiProvider>;

Personally, I think jotai's approach is closer to the provider role that users expect.

My Opinion

If this is the intended design, it would be good to provide documentation on how to test with overlay-kit.

If this wasn't intended, I think we need to think about how to do it.

I'm curious about your opinion,

Thank you for making a great library thanks! 🙂

jungpaeng commented 4 months ago

Hello, and thank you for your interest and questions regarding overlay-kit.

You’re correct in noting that OverlayProviders are not intended to operate independently. The primary purpose of this component is to store state in a ContextAPI Provider and to render overlays alongside children.

This design choice was made with the intent for OverlayProviders to function as a singleton, under the assumption that there wouldn’t be a scenario where OverlayProviders would need to unmount in a service context. However, this overlooks scenarios involving mocking during testing, where OverlayProviders might indeed unmount.

To address this, we will make changes so that overlay.unmountAll() is called automatically when OverlayProviders unmount.

XionWCFM commented 4 months ago

Thank you for your interest in my opinion.

calling unmountAll when OverlayProvider is unmounted seems to solve the problem in the test scenario.

I think writing tests will be more enjoyable once this is done. Thank you.