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: SSR Combobox inner ref lost #7663

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions packages/@react-aria/collections/src/CollectionBuilder.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export interface CollectionBuilderProps<C extends BaseCollection<object>> {
export function CollectionBuilder<C extends BaseCollection<object>>(props: CollectionBuilderProps<C>): ReactElement {
// If a document was provided above us, we're already in a hidden tree. Just render the content.
let doc = useContext(CollectionDocumentContext);
let isSSR = useIsSSR();
if (doc) {
// The React types prior to 18 did not allow returning ReactNode from components
// even though the actual implementation since React 16 did.
Expand All @@ -58,7 +59,7 @@ export function CollectionBuilder<C extends BaseCollection<object>>(props: Colle
{props.content}
</CollectionDocumentContext.Provider>
</Hidden>
<CollectionInner render={props.children} collection={collection} />
<CollectionInner render={props.children} collection={collection} key={isSSR ? 'server' : 'client'} />
</>
);
}
Expand Down Expand Up @@ -116,7 +117,7 @@ function useCollectionDocument<T extends object, C extends BaseCollection<T>>(cr
useLayoutEffect(() => {
document.isMounted = true;
return () => {
// Mark unmounted so we can skip all of the collection updates caused by
// Mark unmounted so we can skip all of the collection updates caused by
// React calling removeChild on every item in the collection.
document.isMounted = false;
};
Expand Down
57 changes: 56 additions & 1 deletion packages/react-aria-components/test/ComboBox.ssr.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
* governing permissions and limitations under the License.
*/

import {screen, testSSR} from '@react-spectrum/test-utils-internal';
import {fireEvent, screen, testSSR} from '@react-spectrum/test-utils-internal';

describe('ComboBox SSR', function () {
it('should render text of default selected key', async function () {
Expand Down Expand Up @@ -40,4 +40,59 @@ describe('ComboBox SSR', function () {
let input = screen.getByRole('combobox');
expect(input.value).toBe('Dog');
});

it('should point ref correctly after hydration', async function () {
await testSSR(__filename, `
import {ComboBox, Label, Input, Popover, ListBox, ListBoxItem} from '../';
import {useState, useRef} from 'react';
import {useLayoutEffect} from '@react-aria/utils';

function App() {
let [triggers, setTriggers] = useState(['null']);
let [otherState, setOtherState] = useState(0);
let ref = useRef(null);

useLayoutEffect(() => {
setTriggers(prev => [...prev, ref.current?.outerHTML]);
}, [otherState]);

return (
<React.StrictMode>
<ComboBox defaultSelectedKey="dog">
<div ref={ref} role="group">
<Label>Favorite Animal</Label>
<Input />
</div>
<Popover>
<ListBox>
<ListBoxItem id="cat">Cat</ListBoxItem>
<ListBoxItem id="dog">Dog</ListBoxItem>
<ListBoxItem id="kangaroo">Kangaroo</ListBoxItem>
</ListBox>
</Popover>
</ComboBox>
<div role="button">{triggers.join(", ")}</div>
<div role="button" onClick={() => setOtherState(1)}>{otherState}</div>
</React.StrictMode>
);
}
<App />
`, () => {
// Assert that server rendered stuff into the HTML.
let input = screen.getByRole('combobox');
expect(input.value).toBe('Dog');
let buttons = screen.getAllByRole('button');
expect(buttons[0].textContent).toBe('null');
});

// Assert that hydrated UI matches what we expect.
let input = screen.getByRole('combobox');
expect(input.value).toBe('Dog');
let buttons = screen.getAllByRole('button');
let [button, button2] = buttons;
fireEvent.click(button2);
expect(button2.textContent).toBe('1');
let [, , second] = button.textContent.split(', ');
expect(second).toBe(screen.getByRole('group').outerHTML);
});
});