facebook / prop-types

Runtime type checking for React props and similar objects
MIT License
4.48k stars 356 forks source link

Support JavaScript standard type ‘BigInt’ #355

Closed bignose-debian closed 2 years ago

bignose-debian commented 3 years ago

The JavaScript standard data type ‘BigInt’ does not have obvious support in PropTypes.

Existing behaviour with Number

React.version  // → "17.0.2"

Foo = function(props) {
    return React.createElement()
}

props = {tally: 100}
typeof(props.tally)  // → "number"
PropTypes.number  // → function checkType()
typeSpecs = {tally: PropTypes.number}
PropTypes.checkPropTypes(typeSpecs, props)  // → undefined (success)

Foo.propTypes = {
    tally: PropTypes.number,
}
React.createElement(Foo, props)  // → Object { "$$typeof": Symbol(react.element), …}, success

Expected behaviour with BigInt

Foo = function(props) {
    return React.createElement()
}

props = {tally: BigInt(100)}
typeof(props.tally)  // → "bigint"
PropTypes.bigint  // → function checkType()
typeSpecs = {tally: PropTypes.bigint}
PropTypes.checkPropTypes(typeSpecs, props)  // → undefined (success)

Foo.propTypes = {
    tally: PropTypes.instanceOf(BigInt),
}
React.createElement(Foo, props)  // → Object { "$$typeof": Symbol(react.element), …}, success

That is, JavaScript reports the type as "bigint", so the expected type specifier is PropTypes.bigint.

Actual behaviour with BigInt

props = {tally: BigInt(100)}
typeof(props.tally)  // → "bigint"
PropTypes.bigint  // → undefined

So while JavaScript reports the type as "bigint", there is no PropTypes.bigint.

Attempting to work around this with PropTypes.instanceOf appears to work:

props = {tally: BigInt(100)}
typeof(props.tally)  // → "bigint"
PropTypes.instanceOf(BigInt)  // → function checkType()
typeSpecs = {tally: PropTypes.instanceOf(BigInt)}
PropTypes.checkPropTypes(typeSpecs, props)  // → undefined (success)

But this fails when React tests a component's props:

React.version  // → "17.0.2"

Foo = function(props) {
    return React.createElement()
}
Foo.propTypes = {
    tally: PropTypes.instanceOf(BigInt),
}

props = {tally: BigInt(100)}
React.createElement(Foo, props)

Warning: Failed prop type: Invalid prop tally of type BigInt supplied to Foo, expected instance of BigInt.

ljharb commented 3 years ago

instanceOf will not work for primitives, so indeed you'd want a PropTypes.bigint.

bignose-debian commented 3 years ago

instanceOf will not work for primitives

It appears to work with a call to PropTypes.checkPropTypes though (see the examples in the initial message). The failure comes when React uses PropTypes.

indeed you'd want a PropTypes.bigint.

That is the ideal resolution, agreed.

ljharb commented 3 years ago

That seems like a bug in checkPropTypes tho - which version of React did you compare it to?

bignose-debian commented 3 years ago

That seems like a bug in checkPropTypes tho - which version of React did you compare it to?

Now updated the examples to show React.version.

ljharb commented 3 years ago

So, that's React 17 - what does React 16 do? This library hasn't been updated for any changes in React 17.

bignose-debian commented 3 years ago

what does React 16 do?

Same behaviour using prop-types 15.7 with React 16:

React.version  // → "16.14.0"

Foo = function(props) {
    return React.createElement()
}
Foo.propTypes = {
    tally: PropTypes.instanceOf(BigInt),
}

props = {tally: BigInt(100)}
React.createElement(Foo, props)

Warning: Failed prop type: Invalid prop tally of type BigInt supplied to Foo, expected instance of BigInt.

bignose-debian commented 3 years ago

This library hasn't been updated for any changes in React 17.

Oh, which library should we use for PropTypes support in React 17 and later?

ljharb commented 3 years ago

There isn’t any other one - it’s just that this one hasn’t been updated for react 17.

Looks like checkPropTypes has a bug.

sumarlidason commented 3 years ago

for the next person,

yourPropName: (props, propName, componentName) => {
  if (typeof props[propName] !== 'bigint') {
    return new Error(`Invalid prop '${propName}' supplied to '${componentName}', expected 'BigInt'.`);
  }
}
ljharb commented 2 years ago

If you observe differences in the way checkPropTypes works versus React, please file a bug on the appropriate library - here if checkPropTypes is clearly wrong, and on React itself if React is clearly wrong.