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

Add new deprecations from 15.5.0 #1148

Merged
merged 4 commits into from Apr 20, 2017

Conversation

Calyhre
Copy link
Contributor

@Calyhre Calyhre commented Apr 11, 2017

Added more deprecations rules regarding last react update.

But it does not seem to match patterns like this:

import { PropTypes, createClass } from 'react';

Any idea on how we can match those too?

@ljharb
Copy link
Member

ljharb commented Apr 11, 2017

(linking to #1144)

ljharb
ljharb previously requested changes Apr 11, 2017
Copy link
Member

@ljharb ljharb left a comment

We definitely also need to handle both destructuring of these properties and named imports.

@ljharb ljharb added the rule label Apr 11, 2017
@yannickcr
Copy link
Member

yannickcr commented Apr 18, 2017

Actually destructuring is not handled for any deprecation in this rule. Mainly because it was not something common at the time it was written. So I think it should not be a blocker for this PR.

But, since it is now a very common pattern, I'm working on a patch to add destructuring support to the whole rule.

@@ -64,6 +64,9 @@ module.exports = {
deprecated.MemberExpression['Perf.printDOM'] = ['15.0.0', 'Perf.printOperations'];
deprecated.MemberExpression['ReactPerf.getMeasurementsSummaryMap'] = ['15.0.0', 'ReactPerf.getWasted'];
deprecated.MemberExpression['Perf.getMeasurementsSummaryMap'] = ['15.0.0', 'Perf.getWasted'];
// 15.5.0
deprecated.MemberExpression['React.createClass'] = ['15.5.0', 'the npm module create-react-class'];
deprecated.MemberExpression['React.PropTypes'] = ['15.5.0', 'the npm module prop-types'];
Copy link
Member

@yannickcr yannickcr Apr 18, 2017

Choose a reason for hiding this comment

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

Please use the pragma variable instead of the hardcoded React string (you can take the other deprecation as example).

Copy link
Contributor Author

@Calyhre Calyhre Apr 18, 2017

Choose a reason for hiding this comment

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

Pragma would match if there is other thing than React.* used right?
I don't think it should match if this is from another object than React.

Copy link
Member

@yannickcr yannickcr Apr 18, 2017

Choose a reason for hiding this comment

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

pragma will match other thing than React only if the user has customized it to match something else (with a comment or using the shared settings).

Copy link
Contributor Author

@Calyhre Calyhre Apr 18, 2017

Choose a reason for hiding this comment

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

Got it 👍 PR updated

@ljharb
Copy link
Member

ljharb commented Apr 18, 2017

@yannickcr i'm fine with not handling destructuring in this PR (especially if you're working on adding it asap), but named imports should be handled, no?

@yannickcr
Copy link
Member

yannickcr commented Apr 19, 2017

@ljharb like destructuring, it's something that was not handled by the rule, so I think it should be done separately from this PR.

But my patch is almost ready, I should submit it tomorrow (I hope we'll be able to ship React 15 support this week).

@ljharb ljharb dismissed their stale review Apr 19, 2017

👍 deferring to @yannickcr

@yannickcr yannickcr merged commit eae04d4 into jsx-eslint:master Apr 20, 2017
3 checks passed
@Calyhre Calyhre deleted the feature/deprecations-react-15.5 branch Apr 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

3 participants