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

no-typos rule doesn't handle custom proptypes classes #1389

Closed
modosc opened this issue Aug 22, 2017 · 25 comments
Closed

no-typos rule doesn't handle custom proptypes classes #1389

modosc opened this issue Aug 22, 2017 · 25 comments

Comments

@modosc
Copy link

modosc commented Aug 22, 2017

with the 7.2.1 -> 7.3.0 upgrade this rule started breaking on this code:

import * as MyPropTypes from 'lib/my-prop-types'

const Foo = ({ bar } = {} ) => <div className={bar} />

Foo.propTypes = {
  bar: MyPropTypes.bar.isRequired
}

with

  29:25  error  Typo in declared prop type: bar  react/no-typos

interestingly enough if i change to this:

Foo.propTypes = {
  bar: MyPropTypes.string.isRequired
}

then the rule passes, even though MyPropTypes doesn't export a string prop.

@jseminck
Copy link
Contributor

Just for information, indeed we added logic to validate the specific PropTypes. Since string is a key that is exported from the official prop-types package, it doesn't fail. bar is not a prop type defined in the official prop-types package, so it fails.

Not sure how we should handle this case yet. Just explaining what changes and why the error is introduced. 😄

@ljharb
Copy link
Member

ljharb commented Aug 22, 2017

This seems like a bug in no-typos - we should only be looking for typos on React.PropTypes or on an import/require of prop-types.

@jseminck
Copy link
Contributor

jseminck commented Sep 12, 2017

I'm trying to work on this... I guess we need to support the following cases? Did I miss anything?

import PropTypes from "prop-types"

import { PropTypes } from "react";

import { PropTypes as ValidPropTypes } from "react"

import React from "react"
React.PropTypes

@ljharb
Copy link
Member

ljharb commented Sep 12, 2017

@jseminck also require('prop-types') in any form.

@martin-svk
Copy link

martin-svk commented Oct 9, 2017

It also fails when importing specific prop types directly:

import { func, number } from 'prop-types';

MyComponent.propTypes = {
    funcProp: func.isRequired,
    numberProp: number.isRequired
};

Resulting in react/no-typos Error.

@oieduardorabelo
Copy link

same happens when using react-intl inject hoc:

import { injectIntl, intlShape } from "react-intl";

function Example() { //code }

Example.propTypes = {
  // ESLint error: 'Typo in declared prop type: isRequired (react/no-typos)'
  intl: intlShape.isRequired
};

export default injectIntl(Example)

screen shot 2017-10-10 at 9 56 00 am

you can ignore the line number in the screenshot, is just an example

@felpin
Copy link

felpin commented Oct 14, 2017

Well, this does not resolve the issue, but you can disable a rule in ESLint with a comment

Foo.propTypes = {
  // eslint-disable-next-line react/no-typos
  bar: MyPropTypes.string.isRequired
}

In this particular case I do that while this issue is open

@neiker
Copy link

neiker commented Oct 17, 2017

Same using react-immutable-proptypes

@zxlin
Copy link

zxlin commented Oct 25, 2017

@jseminck

Another case where webpack's ProvidePlugin is used to avoid repetitively importing prop-types everywhere.

In my code I don't import prop-types explicitly and simply write:

const { string, object } = PropTypes;

and get the validators that way as webpack will replace all instances of PropTypes with the prop-types module.

@ljharb
Copy link
Member

ljharb commented Oct 25, 2017

@zxlin using webpack to get magic implicit imports isn't necessarily a use case we should be handling or encouraging, imo.

@the-noob
Copy link

the-noob commented Nov 2, 2017

Adding this for whoever looks for MobX no-typos error :)

import { observer, inject, PropTypes as MobXPropTypes } from 'mobx-react';

class App extends React.Component { ...}

App.wrappedComponent.propTypes = {
  appStore: MobXPropTypes.objectOrObservableObject.isRequired,
};

Produces

Typo in declared prop type: objectOrObservableObject react/no-typos

@jseminck
Copy link
Contributor

I'm working on this, so that custom PropTypes imports will be ignored and only the real "prop-types" package will be considered for checking typos.

@ljharb
Copy link
Member

ljharb commented Nov 18, 2017

@jseminck also React.PropTypes, hopefully :-)

@Kenjirou
Copy link

Same typo-error for ReactNative's style proptypes:

import PropTypes from 'prop-types';
import {View} from 'react-native';

CoolComponent.propTypes = {
    containerStyle: View.propTypes.style,
};

@abdennour
Copy link

@jseminck , any update ? we are waiting ☕️

@Hypnosphi
Copy link
Contributor

@abdennour you can make your own PR if you feel like it

@jseminck
Copy link
Contributor

jseminck commented Nov 26, 2017

I have a branch here: https://github.com/jseminck/eslint-plugin-react/tree/no-typos-react-import

It still has a failing test - but if you have any ideas feel free to help. If you want to finish it then you're free to take this code and continue working on top of it.

This turned out harder than I thought. Didn't even work with require() yet 😞

Furthermore the React/PropTypes import code should probably be extracted to an util because other rules could also benefit from it.

@extwiii
Copy link

extwiii commented Dec 7, 2017

any update ? we are waiting ☕️

@alex-shamshurin
Copy link

Yes, I have the same issue. It works inside React.component extended class and doesn't work for stateless components

@Koleok
Copy link

Koleok commented Jan 8, 2018

I just need to drop a note on the repeated toe-tapping scoldy comments in this thread

ie:

any update? we are waiting ☕️

Github automatically posts references for:

  • opens/closes/merges on related MRs
  • commits with references to this issue
  • references to this issue from other issues

These are the updates you are looking for.
updates

It is not, and should not be the responsibility of anyone involved in an OSS project to look through each issue every day and copy/paste links to commits or give progress reports to impatient/unmotivated consumers of the library.

The whole point of open source, is that maintainers do things when they can, and contributors who have time step in and fill the gaps when they can.

If you find yourself continually posting things in github issues like

"hey i need this to work right now how long till you fix it!"

then guess what, you my friend are ready to be an open source contributor.

You might say:

"I don't know how this library works!"

well, read the source, and soon you will

"i don't have time my job is relentless and I'm being pressured"

well, so are the maintainers you are pressuring, probably more so!

"No one has committed anything in a long time and i fear the project is abandoned 😱"

well, fork it and do it your way, or just be patient :)

@dkrutsko
Copy link

dkrutsko commented Jan 26, 2018

Hey everyone, I'd like to add two additional pieces of information regarding this issue:

  1. Since PR support for custom propTypes with isRequired in no-typos #1652 it seems that no-typos started detecting more issues! However, While this rule detects "invalid" propTypes defined in functional components, it doesn't work inside classes when defining propTypes through static instance variables.

  2. For this issue specifically, I'm guessing that all that needs to change is this variable? Would mutating it through props solve the issue?

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

No branches or pull requests