Skip to content

Commit

Permalink
Merge branch 'main' into use-id-hook
Browse files Browse the repository at this point in the history
  • Loading branch information
JoviDeCroock committed Apr 17, 2022
2 parents e9c1876 + ac88f1f commit 3175c73
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 12 deletions.
11 changes: 2 additions & 9 deletions compat/src/forwardRef.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,10 @@ export const REACT_FORWARD_SYMBOL =
* @returns {import('./internal').FunctionComponent}
*/
export function forwardRef(fn) {
// We always have ref in props.ref, except for
// mobx-react. It will call this function directly
// and always pass ref as the second argument.
function Forwarded(props, ref) {
function Forwarded(props) {
let clone = Object.assign({}, props);
delete clone.ref;
ref = props.ref || ref;
return fn(
clone,
!ref || (typeof ref === 'object' && !('current' in ref)) ? null : ref
);
return fn(clone, props.ref || null);
}

// mobx-react checks for this being present
Expand Down
43 changes: 42 additions & 1 deletion compat/test/browser/forwardRef.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import React, {
useState,
useRef,
useImperativeHandle,
createPortal
createPortal,
Component
} from 'preact/compat';
import { setupScratch, teardown } from '../../../test/_util/helpers';
import { setupRerender, act } from 'preact/test-utils';
Expand Down Expand Up @@ -457,4 +458,44 @@ describe('forwardRef', () => {
render(App({}, null), scratch);
expect(actual).to.equal(null);
});

// Issue #3483
it('should allow for multiple refs', () => {
let actual;
const App = forwardRef((_, ref) => {
actual = ref;
return <div />;
});

const ref = { a: createRef(), b: createRef() };
// eslint-disable-next-line new-cap
render(<App ref={ref} />, scratch);
expect(actual).to.equal(ref);
});

it('should not leak context into refs', () => {
class Provider extends Component {
getChildContext() {
return { foo: 2 };
}
render() {
return this.props.children;
}
}

let actual;
const Forwarded = forwardRef((_, ref) => {
actual = ref;
return <div />;
});

render(
<Provider>
<Forwarded />
</Provider>,
scratch
);

expect(actual).to.equal(null);
});
});
9 changes: 7 additions & 2 deletions hooks/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,13 @@ export function useImperativeHandle(ref, createHandle, args) {
currentHook = 6;
useLayoutEffect(
() => {
if (typeof ref == 'function') ref(createHandle());
else if (ref) ref.current = createHandle();
if (typeof ref == 'function') {
ref(createHandle());
return () => ref(null);
} else if (ref) {
ref.current = createHandle();
return () => (ref.current = null);
}
},
args == null ? args : args.concat(ref)
);
Expand Down
19 changes: 19 additions & 0 deletions hooks/test/browser/useImperativeHandle.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,4 +179,23 @@ describe('useImperativeHandle', () => {

expect(() => render(<Comp />, scratch)).to.not.throw();
});

it('should reset ref to null when the component get unmounted', () => {
let ref,
createHandleSpy = sinon.spy(() => ({ test: () => 'test' }));

function Comp() {
ref = useRef({});
useImperativeHandle(ref, createHandleSpy, [1]);
return <p>Test</p>;
}

render(<Comp />, scratch);
expect(createHandleSpy).to.have.been.calledOnce;
expect(ref.current).to.not.equal(null);

render(<div />, scratch);
expect(createHandleSpy).to.have.been.calledOnce;
expect(ref.current).to.equal(null);
});
});
3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,9 @@
"jsx-runtime/dist",
"jsx-runtime/src",
"jsx-runtime/package.json",
"server/src",
"server/package.json",
"server/dist",
"test-utils/src",
"test-utils/package.json",
"test-utils/dist"
Expand Down

0 comments on commit 3175c73

Please sign in to comment.