Allow require-default-prop to be set to false for non required props

[filipalacerda]

Hello!

While adding this plugin to our project, I'm finding the need to add configuration to this rule: https://github.com/vuejs/eslint-plugin-vue/blob/master/lib/rules/require-default-prop.js

It is helpful to have a default prop in most cases, but sometimes we use the non existence of one to check if a certain part of a component should be rendered.

For example:
For the non-required string prop title, checking if title exists is enough:

props: {
  title: {
    type: String, 
    required: false,
  },
},
...
<p v-if="title>{{ title }}</p>

If I have to set a default to an empty string, I need an extra check.

Or for a datepicker component that accepts a minDate as a non required prop, it is very helpful to not have a default value, in the cases where a min date is not needed.

This was something that we found useful after starting the style guide: https://docs.gitlab.com/ce/development/fe_guide/style_guide_js.html#props

Default key should be provided if the prop is not required. Note: There are some scenarios where we need to check for the existence of the property. On those a default key should not be provided.

[filipalacerda]

Forgot to mention that I'm more than happy to submit a PR for this if you think it's ok to add this :)

[michalsnik]

Hey @filipalacerda 👋 Thank you for posting this proposition. Could you please give an example how would you want to configure this rule?

Did you think about ingoring specific types like this or have you got something else in mind?

ignoredTypes: [...]

I'm not sure we can easily predict users intention, and perhaps for those who find this rule problematic it's better to just turn it off.

[filipalacerda]

Hi @michalsnik!

I was thinking of configuring only by required or non required:

Both required and non required props need a default value:

{
  "vue/require-default-prop": ["error", {
    "required": true,
    "non-required": true,
  }]
}

// no errors:
{
  props: {
    prop1: {
      type: String,
      required: true,
      default: '',
    },
    prop2: {
      type: Array,
      required: false,
      default: () => [],
    }
  },
}

Required prop needs a default, non required props doesn't need a default value:

{
  "vue/require-default-prop": ["error", {
    "required": true,
    "non-required": false,
  }]
}

// no errors:
{
  props: {
    prop1: {
      type: String,
      required: true,
      default: '',
    },
    prop2: {
      type: Array,
      required: false,
    }
  },
}

None needs a default:

{
  "vue/require-default-prop": ["error", {
    "required": false,
    "non-required": false,
  }]
}

// no errors:
{
  props: {
    prop1: {
      type: String,
      required: true,
    },
    prop2: {
      type: Array,
      required: false,
    }
  },
}

The defaults could be what we have now, true for both. What do you think?

[michalsnik]

Hey @filipalacerda, To be honest I don't really see a value in those settings.

• If the prop is required that means we demand something being passed down in form of a specific prop, so we don't want to require user to set default prop for it in that case, no matter the setting.
• If the prop is not required - it's the only case in which we want to set default value as it might not be passed down in the first place, and if you don't want this to throw a warning you can simply disable this rule.

Or perhaps I'm missing something :)

And as for your initial case, you might also write it like this:

props: {
  title: {
    type: [String, Boolean],
    default: false
  },
},

That should do the job, and you'll be perfectly explicit :) WDYT?

[filipalacerda]

@michalsnik Sorry it took me so long to get back to you

If the prop is not required - it's the only case in which we want to set default value as it might not be passed down in the first place, and if you don't want this to throw a warning you can simply disable this rule.

Yes, this is the approach I followed in the files we are using the fact the prop is null to do some verifications: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/clusters/components/application_row.vue#L2 🙈

And as for your initial case, you might also write it like this:

props: {
title: {
type: [String, Boolean],
default: false
},
},
That should do the job, and you'll be perfectly explicit :) WDYT?

Yes, I think that should also help.

Going to close this issue :) Thanks for the help! 💪