Skip to content

Commit

Permalink
Fix duplicate IDs in different instances (#54)
Browse files Browse the repository at this point in the history
* Adopt newer version of Reach's auto-id

* change from const to function
  • Loading branch information
roginfarrer committed Aug 13, 2020
1 parent 999d310 commit cc8f490
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 13 deletions.
36 changes: 35 additions & 1 deletion src/__tests__/utils.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,41 @@
import React from 'react';
import { useEffectAfterMount, useControlledState, callAll } from '../utils';
import {
useEffectAfterMount,
useControlledState,
callAll,
useUniqueId,
} from '../utils';
import { render, act } from '@testing-library/react';

describe('useUniqueId', () => {
it('should generate a unique ID value', () => {
function Comp() {
const justNull = null;
const randId = useUniqueId(justNull);
const randId2 = useUniqueId();
return (
<div>
<div id={randId}>Wow</div>
<div id={randId2}>Ok</div>
</div>
);
}
const { getByText } = render(<Comp />);
const id1 = Number(getByText('Wow').id);
const id2 = Number(getByText('Ok').id);
expect(id2).not.toEqual(id1);
});

it('uses a fallback ID', () => {
function Comp() {
const newId = useUniqueId('awesome');
return <div id={newId}>Ok</div>;
}
const { getByText } = render(<Comp />);
expect(getByText('Ok').id).toEqual('awesome');
});
});

describe('callAll', () => {
it('it calls the two functions passed into it', () => {
const functionOne = jest.fn();
Expand Down
68 changes: 56 additions & 12 deletions src/utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
import { RefObject, useState, useRef, useEffect, useCallback } from 'react';
import {
RefObject,
useState,
useRef,
useEffect,
useCallback,
useLayoutEffect,
} from 'react';
import warning from 'tiny-warning';
import { AssignableRef } from './types';

Expand Down Expand Up @@ -118,19 +125,56 @@ export function useEffectAfterMount(
}, dependencies);
}

// Unique ID implementation borrowed from React UI :)
// https://github.com/reach/reach-ui/blob/6e9dbcf716d5c9a3420e062e5bac1ac4671d01cb/packages/auto-id/src/index.js
let idCounter = 0;
const genId = (): number => ++idCounter;

/**
* This generates a unique ID for an instance of Collapse
* @return {String} the unique ID
* Taken from Reach
* https://github.com/reach/reach-ui/blob/d2b88c50caf52f473a7d20a4493e39e3c5e95b7b/packages/auto-id
*
* Autogenerate IDs to facilitate WAI-ARIA and server rendering.
*
* Note: The returned ID will initially be `null` and will update after a
* component mounts. Users may need to supply their own ID if they need
* consistent values for SSR.
*
* @see Docs https://reach.tech/auto-id
*/
export function useUniqueId(): number {
const [id, setId] = useState(0);
useEffect(() => setId(genId()), []);
return id;
const useIsomorphicLayoutEffect =
typeof window !== 'undefined' ? useEffect : useLayoutEffect;
let serverHandoffComplete = false;
let id = 0;
const genId = () => ++id;
export function useUniqueId(idFromProps?: string | null) {
/*
* If this instance isn't part of the initial render, we don't have to do the
* double render/patch-up dance. We can just generate the ID and return it.
*/
const initialId = idFromProps || (serverHandoffComplete ? genId() : null);

const [id, setId] = useState(initialId);

useIsomorphicLayoutEffect(() => {
if (id === null) {
/*
* Patch the ID after render. We do this in `useLayoutEffect` to avoid any
* rendering flicker, though it'll make the first render slower (unlikely
* to matter, but you're welcome to measure your app and let us know if
* it's a problem).
*/
setId(genId());
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

useEffect(() => {
if (serverHandoffComplete === false) {
/*
* Flag all future uses of `useId` to skip the update dance. This is in
* `useEffect` because it goes after `useLayoutEffect`, ensuring we don't
* accidentally bail out of the patch-up dance prematurely.
*/
serverHandoffComplete = true;
}
}, []);
return id != null ? String(id) : undefined;
}

export function usePaddingWarning(element: RefObject<HTMLElement>): void {
Expand Down

0 comments on commit cc8f490

Please sign in to comment.