Skip to content

fix(require-default-prop): avoid requiring defaults for optional props not included in destructuring #2768

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

yuasa-engineer
Copy link

@yuasa-engineer yuasa-engineer commented Jun 24, 2025

Fixes #2767

If this approach doesn't align with the project's direction, please don't hesitate to reject this proposal. In that case, you may also close the corresponding issue.

What type of PR is this?

bug fix / enhancement

What does this PR do? / Why do we need it?

This PR fixes a regression in the vue/require-default-prop rule introduced in v10.1.0. The rule was requiring default values for ALL optional props when using props destructuring, even if those props were not included in the destructuring pattern.

Before this fix

// Was forced to destructure all optional props, even unused ones
const { used, unused = undefined } = defineProps<{
  used?: string;
  unused?: string; // Only used in template, forced to assign undefined
}>()

After this fix

// Only need to provide defaults for actually destructured optional props
const { used = 'default' } = defineProps<{
  used?: string;    // Destructured, so needs default
  unused?: string;  // Not destructured, no default needed
}>()

What changes did you make? (Give an overview)

  1. Modified lib/rules/require-default-prop.js:

    • Added logic to check if an optional prop is actually included in the destructuring pattern
    • Only requires default values for optional props that are explicitly destructured
    • Uses utils.getPropsDestructure(node) to get destructured props and check if optional prop exists
  2. Updated test cases in tests/lib/rules/require-default-prop.js:

    • Added valid test cases for optional props not included in destructuring
    • Updated invalid test cases to properly test mixed scenarios
    • Added comprehensive test coverage for the new behavior

Which issue(s) does this PR fix?

Fixes #2767
Related to #2725 and #2741

Additional context

This change maintains backward compatibility and only reduces false positive warnings. The rule continues to work as expected for:

  • Required props (still checked)
  • Optional props with explicit destructuring (still requires defaults)
  • Boolean props (still exempt)
  • Props using withDefaults() (still works)

The fix specifically addresses the user feedback that the previous fix was too aggressive and made Vue 3.5's props destructuring unnecessarily verbose.

…s not included in destructuring

- Only require default values for optional props that are explicitly destructured
- Optional props not included in destructuring pattern are now ignored
- Fixes regression introduced in v10.1.0 where all optional props required defaults
Copy link

changeset-bot bot commented Jun 24, 2025

🦋 Changeset detected

Latest commit: 88b94f2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
eslint-plugin-vue Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@waynzh
Copy link
Member

waynzh commented Jul 1, 2025

Sorry for the late reply.

I'm not sure about others' use cases. For example, in the previous withDefaults case, any optional fields without a defined default value would result in errors. So why are we choosing to ignore this error specifically when destructuring props?

withDefaults(defineProps<{
  used?: string;
  unused?: string;  // error! Prop 'unused' requires default value to be set.
}>(), {
  used: 'default'
})

It looks like there's some feedback about this being too verbose. Maybe we could add an option to control whether optional props not included in destructuring should be ignored? This way, users who know what they're doing can explicitly configure it.

I'll hold off for now and see what others think.

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

Successfully merging this pull request may close these issues.

vue/require-default-prop: Avoid requiring defaults for optional props not included in destructuring
2 participants