Skip to content

[New] add prefer-exact-props rule #1547

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

Merged
merged 1 commit into from
Aug 9, 2021
Merged

Conversation

jomasti
Copy link
Contributor

@jomasti jomasti commented Nov 19, 2017

This rule checks if non-exact objects are used for defining prop types when using Flow along with using normal objects for prop types instead of something like prop-types-exact to enforce only passing the exact props to a component.

The exact prop wrapper functions for non-Flow usage are configurable by adding the exact property to an object setting in the propWrapperFunctions config option.

Closes #1455

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Also note the propWrapperFunctions settings, with can be used to specify exact or forbidPropTypes.

Perhaps this warrants a new setting for "exact propType wrappers", that could also be checked everywhere the existing propWrappers are?

@jomasti
Copy link
Contributor Author

jomasti commented Nov 19, 2017

Do you think it should be a global setting instead of just an option for the rule? I'm unsure of the other use cases for it being a global setting.

@ljharb
Copy link
Member

ljharb commented Nov 19, 2017

@jomasti if it were a global setting, then other rules could access it to "unwrap" propTypes for their own uses.

@jomasti
Copy link
Contributor Author

jomasti commented Nov 19, 2017

@ljharb Wouldn't these particular "exact props" functions still be in propWrapperFunctions in order to facilitate that?

@ljharb
Copy link
Member

ljharb commented Nov 19, 2017

@jomasti right; the idea is that i'd be able to move forbidExtraProps etc out of propWrapperFunctions and into this new setting.

@ljharb
Copy link
Member

ljharb commented Nov 19, 2017

I guess an alternative schema for the existing setting could be:

"propWrapperFunctions": [
  "Object",
  { "property": "freeze", "object": "Object" },
  { "property": "forbidExtraProps", "exact": true }
]

@maxammann
Copy link

What exactly is missing before this can be merged? Can we maybe split this task and add support for libs like airbnb/prop-types in an other task?

@ljharb
Copy link
Member

ljharb commented Sep 11, 2018

@maxammann that support already exists; see the "propWrapperFunctions" setting in the readme.

This PR needs to be updated per the above comments.

@jomasti
Copy link
Contributor Author

jomasti commented Dec 4, 2018

After doing some digging, it looks like the failures on ESLint 3 are due to versions 7.22+ of babel-eslint using eslint-scope over escope if available. This combination of the two results is weird errors for TypeAlias and maybe others. I'm not sure what I should do to rectify this.

@ljharb
Copy link
Member

ljharb commented Dec 4, 2018

We could modify the dep range, and for eslint 3 in Travis.yml manually downgrade it?

@jomasti
Copy link
Contributor Author

jomasti commented Dec 4, 2018

We could modify the dep range, and for eslint 3 in Travis.yml manually downgrade it?

Would you prefer for the before_script to be extracted to a separate script with this extra logic?

@jomasti
Copy link
Contributor Author

jomasti commented Dec 4, 2018

Downgrading to a working version of babel-eslint breaks other tests, so it doesn't seem viable.

@ljharb
Copy link
Member

ljharb commented Dec 4, 2018

Breaks other tests on eslint 3?

@jomasti
Copy link
Contributor Author

jomasti commented Dec 4, 2018

Yes, using [email protected] with eslint@3 breaks tests that use object spread in type aliases and the short syntax for fragments.

@ljharb
Copy link
Member

ljharb commented Dec 4, 2018

So what is it exactly about this pr that causes this breakage?

@jomasti
Copy link
Contributor Author

jomasti commented Dec 4, 2018

It's looking for the TypeAlias variable in the scope. This works fine on eslint 4 and above because both it and babel-eslint use eslint-scope.

@jomasti jomasti closed this Jan 4, 2019
@jomasti jomasti changed the title New rule: prefer-exact-props New rule: prefer-exact-prop Jan 4, 2019
@jomasti jomasti changed the title New rule: prefer-exact-prop New rule: prefer-exact-props Jan 4, 2019
@jomasti jomasti reopened this Jan 4, 2019
@jomasti jomasti force-pushed the issue-1455 branch 2 times, most recently from ecbb05d to cb6aba0 Compare January 4, 2019 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Add a rule prefer-exact-props
7 participants