Closed Bug 1176933 Opened 5 years ago Closed 5 years ago

Enable jsx prop types rule for eslint (ensure prop types are specified for used props)

Categories

(Hello (Loop) :: Client, defect)

defect
Not set
Points:
2

Tracking

(Not tracked)

RESOLVED FIXED
Iteration:
41.3 - Jun 29
Blocking Flags:
backlog tech-debt

People

(Reporter: standard8, Assigned: standard8)

References

Details

Attachments

(1 file, 1 obsolete file)

We should enable the prop types rule for eslint, as this will ensure all our props are listed in the component definition.
Flags: qe-verify-
Flags: firefox-backlog+
Relies on the patch chain for bug 1176780.
Attachment #8625524 - Flags: review?(mdeboer)
Attachment #8625524 - Flags: review?(dmose)
Complete patch
Attachment #8625525 - Flags: review?(mdeboer)
Attachment #8625525 - Flags: review?(dmose)
Attachment #8625524 - Attachment is obsolete: true
Attachment #8625524 - Flags: review?(mdeboer)
Attachment #8625524 - Flags: review?(dmose)
Comment on attachment 8625525 [details] [diff] [review]
Enable missing props validation elint rule checking for Loop.

Review of attachment 8625525 [details] [diff] [review]:
-----------------------------------------------------------------

r=dmose, with any appropriate comments addressed.

::: browser/components/loop/content/js/contacts.jsx
@@ +337,5 @@
>      propTypes: {
>        notifications: React.PropTypes.instanceOf(
> +        loop.shared.models.NotificationCollection).isRequired,
> +      // Callback to handle entry to the add/edit contact form.
> +      startForm: React.PropTypes.func.isRequired

Indentation error.

@@ +628,5 @@
>      mixins: [React.addons.LinkedStateMixin],
>  
>      propTypes: {
> +      mode: React.PropTypes.string,
> +      selectTab: React.PropTypes.func.isRequired

Add a comment describing what this means?

::: browser/components/loop/content/js/panel.jsx
@@ +19,5 @@
>  
>    var TabView = React.createClass({
>      propTypes: {
>        buttonsHidden: React.PropTypes.array,
> +      children: React.PropTypes.element,

Should this be isRequired?

::: browser/components/loop/content/shared/js/feedbackViews.jsx
@@ +22,5 @@
>     * -
>     */
>    var FeedbackLayout = React.createClass({
>      propTypes: {
> +      children: React.PropTypes.element,

Why no longer required?
Attachment #8625525 - Flags: review?(mdeboer)
Attachment #8625525 - Flags: review?(dmose)
Attachment #8625525 - Flags: review+
(In reply to Dan Mosedale (:dmose) - needinfo? me for response from comment #3)
> ::: browser/components/loop/content/shared/js/feedbackViews.jsx
> @@ +22,5 @@
> >    var FeedbackLayout = React.createClass({
> >      propTypes: {
> > +      children: React.PropTypes.element,
> 
> Why no longer required?

FeedbackReceived#render doesn't always return elements for standalone, and hence this throws a warning.
Didn't got marked when landed - https://hg.mozilla.org/mozilla-central/rev/fa660238a158
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.