React: React.PropTypes.shape... WTF?

[mbrio]

Sorry for the title, but it was a WTF moment for me when I found that to compare a property to an object schema that the method call was shape. When you read your documentation it makes only slightly more sense than when you see it in code, which is to say it makes no sense when you see it in code.

Anyway, I know this isn't the highest priority item in the world but I would love to request a name change to this method, or at least a synonym that makes more sense like schema, or really anything other than shape.

[probablyup]

:+1: not very intuitive IMO

[bloodyowl]

schema({}) wouldn't be much better IMO, objectOfShape({}) or objectOfStructure({}) would be more expressive.

[mczepiel]

Shape is certainly understood in some circles; it's entered the parlance through V8's optimizations of sharing maps for objects with the same shape.

There's plenty of advice for code to not change shape to benefit from speed increases so I wouldn't be surprised if you see the term thrown around more in the future.

I'm not necessarily advocating for shape, just suggesting its origins. FWIW I think schema feels foreign in this context.

[probablyup]

What about .ofStructure({}) or matchingStructure({}) - I like @bloodyowl's objectOfStructure({}) as well

[mbrio]

@mczepiel I understand why and how the terminology is being used but from the standpoint of the API Facebook is making special effort to ensure that the readability of React code is instantly understood.

I would venture to guess that most front-end JS developers have not looked through V8's optimization documents because they're too busy trying to get their applications working across all the other browsers (Node and advanced front-end developers _MAY_ be more intimate with the terminology though).

I completely agree with you in-terms of schema, but it makes more sense to more people at first glance. Many of the other suggestions above are superior to both schema or shape and to just throw out other suggestions, conformsTo or objectLike would make better terminology at first glance.

[jordansexton]

Most of these suggestions definitely seem more sensible than shape. I use shapes to describe all the models in my application, and it still feels weird and foreign every time I see it in code. I like .objectOfStructure or .objectOfSchema. A shorter descriptor would be nice, but objectLike seems vague to me.

[mbrio]

I completely agree with you @jordansexton on my suggestions, they are not good, but as you state objectOfStructure and objectOfSchema are a bit big.

To be honest I think objectOf would have been better used for this. I think even changing it to shapeOf would be easier to understand, every time I see shape by itself I want to write shape('rectangle').

[jordansexton]

I agree with @mbrio, the current objectOf doesn't seem very useful. There are few applications I can think of where an object could have properties of only one type, and even if they were, the properties would better be described by a shape with declared names just so they'd be properly documented.

Considering that, another option would be making objectOf check to see of the value provided is a PropType or an object. If it's a PropType, the current behavior of .objectOf is used, and the object's properties must be of the declared type. If it's an object, it acts as .shape.

[mbrio]

I'm with you @jordansexton on making the objectOf method take a shape or a PropType, I'll go ahead and create a fork and see if they'd pull that change in.

[mbrio]

@jordansexton it looks like that's a no-go, the objectOf function is a lot more complex than it immediately seems as you can pass other PropTypes into it, for instance you can ensure that the shape of each property matches a PropTypes.shape, you can see this in their tests: https://github.com/facebook/react/blob/master/src/core/__tests__/ReactPropTypes-test.js#L454.

If you ask me the objectOf method is incorrectly named as well, I think the current shape method should have been called objectOf and the current objectOf should be something like propertiesOf.

[jordansexton]

As far as I can see, the method I proposed would still work. As long as what's passed to .objectOf is a type, use the current behavior. This still supports the complex cases shown in those tests. Otherwise, if it's an object literal or other non-PropType object, it's used as a shape descriptor.

[jordansexton]

Naive, untested implementation:

function createObjectOfTypeChecker(typeChecker) {
  if (typeof typeChecker === 'function') {
    // current behavior
    function validate(props, propName, componentName, location) {
      // ...
    }
    return createChainableTypeChecker(validate);
  }
  else {
    // new behavior
    return createShapeTypeChecker(typeChecker);
  }
}

[mbrio]

@jordansexton what I meant by my previous comment was that I don't believe objectOf should be re-used to do both jobs; one that checks that an object conforms to a specific schema and one that ensures each property of an object conforms to a specific schema. I believe that both functions are warranted and needed in their current incarnations. By combining both functions into one I believe it would make it more confusing than the use of the word shape.

Looking back at my suggestions for function names and what I have written in this comment, I'm inclined to suggest objectOf be renamed/aliased to propertiesConformTo and shape be renamed/aliased to objectConformsTo.

The only other suggestion I'd have is to get rid of both of these functions and create a new function that allows for wildcards:

# Checks to ensure all properties are numbers
React.PropTypes.objectConformsTo { '*': React.PropTypes.number }

# Checks to ensure that the object has properties a and b that are numbers
React.PropTypes.objectConformsTo { a: React.PropTypes.number, b: React.PropTypes.number }

Where this gets confusing is in how it handles wildcards mixed in with properties:

# Checks to ensure property a is an object and all other properties are numbers
React.PropTypes.objectConformsTo { '*': React.PropTypes.number, a: React.PropTypes.object }

Not sure how I feel about this, it may be over engineering the validation system for props.

[zpao]

I don't think we're going to change this but I'll let @sebmarkbage make that final decision.

[sebmarkbage]

I can understand how this might be confusing at first. I guess it's just one of those terms that you have to learn. I'm not sure that this clarification is worth churning people that are already using it and accustomed to these terms. Generally I'd be open to improving terminology because most people that will use React haven't started yet. But I predict that a lot of them will use Flow, TypeScript or another dynamic type syntax (guards).

Out of curiosity, what do you guys think about Flow and TypeScript's syntax for object shapes? http://flowtype.org/docs/objects.html

{ [key:string]:number } to ensure that all properties are numbers. It's kind of weird too but once you learn it, maybe it makes sense?

[crisu83]

+1 for renaming shape to something more understandable. I was as mind-blown as @mbrio when I finally learned the meaning of shape in React.