Skip to content

Commit

Permalink
fix(reactstrap#1289): prevent error in Strict Mode (reactstrap#2672)
Browse files Browse the repository at this point in the history
* fix(reactstrap#1289): prevent error in Strict Mode

- remove findDomNode error which was thrown for components using
Fade.js file in ReactStrict mode

* fix: rule of hooks
  • Loading branch information
illiteratewriter committed Mar 2, 2023
1 parent d0f188b commit 2695bfa
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 37 deletions.
8 changes: 5 additions & 3 deletions src/Fade.js
@@ -1,4 +1,4 @@
import React from 'react';
import React, { useRef } from 'react';
import PropTypes from 'prop-types';
import classNames from 'classnames';
import { Transition } from 'react-transition-group';
Expand Down Expand Up @@ -42,22 +42,24 @@ const defaultProps = {
};

function Fade(props) {
const ref = useRef(null);

const {
tag: Tag,
baseClass,
baseClassActive,
className,
cssModule,
children,
innerRef,
innerRef = ref,
...otherProps
} = props;

const transitionProps = pick(otherProps, TransitionPropTypeKeys);
const childProps = omit(otherProps, TransitionPropTypeKeys);

return (
<Transition {...transitionProps}>
<Transition nodeRef={innerRef} {...transitionProps}>
{(status) => {
const isActive = status === 'entered';
const classes = mapToCssModules(
Expand Down
50 changes: 16 additions & 34 deletions src/__tests__/Alert.spec.js
Expand Up @@ -8,6 +8,7 @@ import { Alert } from '..';
describe('Alert', () => {
beforeEach(() => {
jest.resetModules();
jest.useFakeTimers();
});

it('should render children', () => {
Expand Down Expand Up @@ -104,36 +105,15 @@ describe('Alert', () => {
});

it('should have default transitionTimeouts', () => {
jest.doMock('react-transition-group', () => ({
Transition: jest.fn(() => null),
}));
// eslint-disable-next-line global-require
const { Transition } = require('react-transition-group');
// eslint-disable-next-line global-require
const { Alert } = require('..');
render(<Alert>Hello</Alert>);

render(<Alert>Yo!</Alert>);
expect(screen.getByText(/hello/i)).not.toHaveClass('show');

expect(Transition).toHaveBeenCalledWith(
expect.objectContaining({
timeout: 150,
appear: true,
enter: true,
exit: true,
}),
{},
);
jest.advanceTimersByTime(150);
expect(screen.getByText(/hello/i)).toHaveClass('show');
});

it('should have support configurable transitionTimeouts', () => {
jest.doMock('react-transition-group', () => ({
Transition: jest.fn(() => null),
}));
// eslint-disable-next-line global-require
const { Transition } = require('react-transition-group');
// eslint-disable-next-line global-require
const { Alert } = require('..');

render(
<Alert
transition={{
Expand All @@ -143,18 +123,20 @@ describe('Alert', () => {
exit: false,
}}
>
Yo!
Hello
</Alert>,
);

expect(Transition).toHaveBeenCalledWith(
expect.objectContaining({
timeout: 0,
appear: false,
enter: false,
exit: false,
}),
{},
expect(screen.getByText(/hello/i)).toHaveClass('show');
});

it('works with strict mode', () => {
const spy = jest.spyOn(console, 'error');
render(
<React.StrictMode>
<Alert>Hello</Alert>
</React.StrictMode>,
);
expect(spy).not.toHaveBeenCalled();
});
});
10 changes: 10 additions & 0 deletions src/__tests__/Modal.spec.js
Expand Up @@ -1105,4 +1105,14 @@ describe('Modal', () => {
user.tab();
expect(screen.getByText(/click 1/i)).toHaveFocus();
});

it('works with strict mode', () => {
const spy = jest.spyOn(console, 'error');
render(
<React.StrictMode>
<Modal isOpen>Hello</Modal>
</React.StrictMode>,
);
expect(spy).not.toHaveBeenCalled();
});
});

0 comments on commit 2695bfa

Please sign in to comment.