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

Fix false positive for unknown emits definition in vue/require-explicit-emits rule #1756

Merged
merged 2 commits into from Jan 8, 2022

Conversation

ota-meshi
Copy link
Member

fixes #1748

Copy link
Member

@FloEdelmann FloEdelmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I haven't looked thoroughly at the logic changes in utils though. I noticed two minor code simplifications, but they are not merge-blocking.

}

return getComponentPropsFromDefine(propsNode.value)
return getComponentPropsFromOptions(componentObject)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only statement in the getComponentProps function now. Can we rename it to getComponentPropsFromOptions (also everywhere it is used) and only export a reference to getComponentPropsFromOptions?

module.exports = {
  // …
  getComponentPropsFromOptions,
  // …
}

function getComponentPropsFromOptions() {  }

Or rename getComponentPropsFromOptions to getComponentProps and export only a reference to it?

module.exports = {
  // …
  getComponentProps,
  // …
}

function getComponentProps() {  }

Or don't rename anything, but still only export a function reference?

module.exports = {
  // …
  getComponentProps: getComponentPropsFromOptions,
  // …
}

function getComponentPropsFromOptions() {  }

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your review! I changed method name and to use the shorthand property.
However, for backward compatibility, it also defines a method with the old name.

}

return getComponentEmitsFromDefine(emitsNode.value)
return getComponentEmitsFromOptions(componentObject)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thing here.

Copy link
Member

@FloEdelmann FloEdelmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for implementing the feedback!

@ota-meshi ota-meshi merged commit 834411e into master Jan 8, 2022
@ota-meshi ota-meshi deleted the issue1748 branch January 8, 2022 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

vue/require-explicit-emits doesn't support non-inline props declarations
2 participants