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 issue with isValidElementType invariant check crashing on valid t… #1122

Merged
merged 1 commit into from Dec 13, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions CONTRIBUTING.md
Expand Up @@ -42,12 +42,12 @@ npm run test

To run in explicit React versions (the number is the version, so `test:16.3` will run in React version `16.3`):
```
REACT=16.4 npm run test:ci
REACT=16.4 npm run test
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is no test:ci script

```

To run tests in all supported React versions, `16.4`, 16.5`,
```
REACT=all npm run test:ci
REACT=all npm run test
```

To continuously watch and run tests, run the following:
Expand Down
42 changes: 21 additions & 21 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 11 additions & 1 deletion src/components/connectAdvanced.js
Expand Up @@ -5,6 +5,14 @@ import { isValidElementType } from 'react-is'

import { ReactReduxContext } from './Context'

const stringifyComponent = Comp => {
try {
return JSON.stringify(Comp)
} catch (err) {
return String(Comp)
}
}

export default function connectAdvanced(
/*
selectorFactory is a func that is responsible for returning the selector function used to
Expand Down Expand Up @@ -86,7 +94,9 @@ export default function connectAdvanced(
invariant(
isValidElementType(WrappedComponent),
`You must pass a component to the function returned by ` +
`${methodName}. Instead received ${JSON.stringify(WrappedComponent)}`
`${methodName}. Instead received ${stringifyComponent(
WrappedComponent
)}`
)
}

Expand Down
14 changes: 14 additions & 0 deletions test/components/connect.spec.js
Expand Up @@ -2604,5 +2604,19 @@ describe('React', () => {
)(Comp)
}).toThrow(/renderCountProp is removed/)
})

it('should not error on valid component with circular structure', () => {
const createComp = Tag => {
const Comp = React.forwardRef(function Comp(props) {
return <Tag>{props.count}</Tag>
})
Comp.__real = Comp
return Comp
}

expect(() => {
connect()(createComp('div'))
}).not.toThrow()
})
})
})
5 changes: 4 additions & 1 deletion test/run-tests.js
Expand Up @@ -27,4 +27,7 @@ if (version.toLowerCase() === 'all') {
}
}

npmRun.execSync(`jest -c '${JSON.stringify(jestConfig)}'`, { stdio: 'inherit' })
npmRun.execSync(
`jest -c '${JSON.stringify(jestConfig)}' ${process.argv.slice(2).join(' ')}`,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this has actually allowed me to use --watch (which is still not ideal because we have to edit copy of tests rather than real test files to benefit from that, but better this now than nothing)

{ stdio: 'inherit' }
)