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

QUESTION: Why null and undefined are treated differently for non required props #178

Open
gustaff-weldon opened this issue Oct 12, 2018 · 0 comments

Comments

@gustaff-weldon
Copy link

Question

Prop-types atm treats undefined value differently from a null value for non isRequired fields. I'd like to know if there was any specific reason for that behaviour?

Current status

There are multiple checks that assume property value to be anything other than null before checking if field isRequired:

  let valid = Object.keys(typeDefs).every((key) => {
    const typeDef = typeDefs[key]

    const objectValue = get(value, key)
    if (objectValue === undefined) {
      if (!typeDef.required) {
        return true
      } else {
        if (logErrors) {
          logger.warn(ctx, `Property ${name} is missing required property ${key}`, throwErrors)
        }
        return false
      }
    }

    return validators[typeDef.type](ctx, `${name}.${key}`, objectValue, typeDef, logErrors, throwErrors)
  })

This is done on validateProperty level (https://github.com/ciena-blueplanet/ember-prop-types/blob/master/addon/mixins/prop-types.js#L41) as well as in shape validator (https://github.com/ciena-blueplanet/ember-prop-types/blob/master/addon/utils/validators/shape.js#L35)

As a result, all optional fields must be marked in definition as:

optionalProperty: PropTypes.oneOfType([
    PropTypes.null,
    PropTypes.string
]),

If model happens to have multiple of those this becomes a bit bloated and confusing

Motivation

Our API returns optional fields as null and those must be handled differently from undefined values on prop-types level.

Proposal

Allow optional fields to be either undefined or null without going the extra mile of oneOfType.
Although from the outside it looks like a reasonable default, there might be reasons or cases I'm missing, so I would be ok with a configuration prop to enable that behaviour as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant