feat: useOverlay

[dev-hobin]

Description

Use useOverlay to handle dynamic props and make the overlay behave like a controlled component.

Changes

• Add an UPDATE_CONTEXT event to overlayReducer.
• Add a context property to OverlayItem and make it generic.
• Add a useOverlay hook to synchronize context and provide a function to handle a single overlay that responds to dynamic props.

Motivation and Context

When I first used overlay-kit, I struggled because the state wasn't reflected as expected. I later realized that overlay-kit doesn't directly provide a way to bind dynamic states. This made me wonder if I could contribute to improving it.

How Has This Been Tested?

function DemoWithUseOverlay() {
  const [value, setValue] = useState('');
  const overlay = useOverlay({ context: { value, setValue }, overlayId: '123' });

  return (
    <div>
      <p>Demo with useOverlay value: {value}</p>
      <input type="text" value={value} onChange={(e) => setValue(e.target.value)} />
      <button
        onClick={() => {
          overlay.open(({ isOpen, close, unmount, context }) => {
            const { value, setValue } = context;

            return (
              <Modal isOpen={isOpen} onExit={unmount}>
                <div
                  style={{ display: 'flex', flexDirection: 'column', alignItems: 'center', justifyContent: 'center' }}
                >
                  <p>MODAL CONTENT</p>
                  <input type="text" value={value} onChange={(e) => setValue(e.target.value)} />
                  <button onClick={close}>close modal</button>
                  <p>{JSON.stringify(context, null, 2)}</p>
                </div>
              </Modal>
            );
          });
        }}
      >
        open modal
      </button>
    </div>
  );
}

Screenshots (if appropriate):

Types of changes

• 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 change)
• Documentation update

Checklist

• I have performed a self-review of my own code.
• My code is commented, 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.

Further Comments

I was wondering if Toss team had considered this approach but decided not to provide it for some reason (such as bugs, side effects, usability issues, etc.). If so, I’d love to understand the reasoning behind that decision.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 4070a9a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
overlay-kit	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Mar 12, 2025 3:30am

[jungpaeng]

First of all, thank you for your contribution.

You asked why the Toss team did not adopt this approach, and there are likely a few reasons:

1. From the perspective of closures, it is natural that an overlay component does not track external state changes.
2. There have been very few cases where this need arose, and in most cases, the issue could be resolved by allowing overlay components to function in an uncontrolled state.

For these reasons, detecting external state changes was not a priority for overlay components.
However, while it wasn’t a priority, it would certainly be beneficial if an overlay component could detect external state changes.

I have reviewed the PR you worked on.

It appears to function by using a hook to receive the context, detecting changes in the context via useEffect, updating the overlay state accordingly, and then exposing it back through a callback.

If the context contains a primitive value, this wouldn’t be an issue. However, if it holds an object or an array, there could be a problem where useEffect keeps triggering continuously.
Additionally, since there is no way to distinguish whether a value is a getter or a setter, if you process and pass a function instead of directly passing setState, the same issue may occur.

It would be great to find an effective way to resolve this issue.

I also have an additional question.
Since the overlay options type has been updated, it seems that the second argument (context) can now be passed even when calling overlay.open directly, without using useOverlay.
However, due to JavaScript’s nature, in this case, even if context.setValue is called, it seems that the change would not be recognized.
Was this intentional, or was it an unintended addition?

[dev-hobin]

Thank you for your review, and also for sharing why the Toss team did not adopt this approach.

Perhaps I need to adapt to using overlay-kit only in an uncontrolled state manner.

Here is my response to the review.

Passing a reference value inside the context does not cause useEffect to be continuously triggered. No matter what value is passed, the state always flows from OverlayProvider downwards, so it should be referring to the same reference value. In fact, in the example code, the context being passed to the useOverlay hook is also a reference value. Are there any edge cases I might not have considered?

Additionally, I believe it is up to the user to determine whether a value is a getter, a setter, or a function used inside an effect (i.e., whether its reference should remain stable). They can also use useCallback or useMemo for values passed to the overlay if needed. Fundamentally, the considerations when passing values into React Context remain the same in this case.

The second argument (context) of overlay.open was an unintended addition.
While implementing useOverlay, I wanted to hide the context property in the second argument of overlay.open, but it turned out to be more challenging than expected.

To make meaningful use of that option without modifying the code too much, one possible approach would be to rename the variable to initialContext and use it similarly to setting a default value in an uncontrolled component.

[jungpaeng]

Hello, I read your response carefully.
First of all, I am using a translation tool, so my writing may not be conveyed smoothly. I would appreciate your understanding on this.

The issue I mentioned about useEffect being unnecessarily triggered can occur in the following situation:

const [data, setData] = useState({
  name: 'test',
  age: 0,
  profile: {
    avatar: 'https://randomuser.me/api/portraits/men/1.jpg',
    bio: 'Hello!',
    joinDate: '2023-05-15',
  },
});

const overlay = useOverlay({
  context: { profile: data.profile },
});

...

// Outside the overlay component
<button onClick={() => setData({ ...data, age: data.age - 1 })}>-</button>

If the state is structured as an object, and only a part of the object is passed as context,
whenever the external state data(ex. data.age) is updated outside the overlay,
the useEffect inside useOverlay will be executed again.

Since useEffect only contains overlay.updateContext, and overlay.updateContext itself does very little,
this may not cause an issue for now.
However, there is no guarantee that overlay-kit will remain the same after multiple updates.

As you mentioned, one approach is to shift this responsibility from overlay-kit to developers using overlay-kit.
The current behavior of overlay-kit, which operates in an uncontrolled state manner, can also be seen in this light.

The issue I brought up can be avoided if developers properly use useMemo and useCallback.
However, I believe that managing overlays with an uncontrolled state is a cheaper approach than requiring developers to use useMemo and useCallback for proper state isolation when using context.

You used the ContextAPI as an example.
The ContextAPI allows splitting data across multiple providers when necessary.
However, the context of useOverlay seems less suitable for being split across multiple layers,
so I think it differs from the ContextAPI in nature.

That said, I completely understand the need to manage overlays in a controlled state,
and I would like to explore whether there could be a better API design.

Until then, would it be okay to postpone the review of this PR?

[dev-hobin]

Yes, of course. I understand what you're saying. I look forward to a better API design.