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

feat(props): allow defining a required prop as null #9358

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

posva
Copy link
Member

@posva posva commented Jan 23, 2019

Based on #1961. It wasn't possible to specify a required prop of type object and pass null as the
value

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:

The tests cases reflect what I'm trying to fix. I remember this was pointed out multiple times in the past. We cannot explicitely pass a null prop when the type is Object and the prop is required. This would keep current behaviour but allow having the type null for explicit null values. I want to add more tests and maybe refactor but I remember this was problematic as it could break some validation, so I would like so feedback about types you may use that would break with this

Based on vuejs#1961. It wasn't possible to specify a required prop of type object and pass null as the
value
@Justineo
Copy link
Member

Some potential inconsistency here:

props: {
  foo: null, // skip
  bar: {
    type: null // skip
  },
  baz: {
    type: [String, null] // String or null
  }
}

While using constructors, all three usage share the same semantics.

@posva
Copy link
Member Author

posva commented Jan 23, 2019

Added some tests to reflect that as well

@dilraj-vidyard
Copy link

dilraj-vidyard commented Mar 8, 2023

There are tests for this now. Is there still something preventing this from going in?

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

Successfully merging this pull request may close these issues.

4 participants