Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Allow camelcased deprecated lifecycle methods prepended with UNSAFE #57

Closed
wants to merge 1 commit into from

Conversation

kngroo
Copy link
Contributor

@kngroo kngroo commented Dec 20, 2018

To:

@udemy/team-f

What:

In order to upgrade React past version 16.3, the react/no-unsafe eslint rule must be satisfied.

However, that rule conflicts with camelcase without providing the allow option to the rule.

This PR configures the camelcase rule to allow certain UNSAFE_ prefixed deprecated React component lifecycle methods to be exempt.

Reference: eslint/eslint#10783

React 16.7 Upgrade PR: https://github.com/udemy/website-django/pull/31054

JIRA:
What did you test:

<Any manual testing you've done in addition to the automated tests>

What dashboards will you be monitoring:

@kngroo kngroo requested a review from a team December 20, 2018 09:07
Copy link
Contributor

@cansin cansin left a comment

Choose a reason for hiding this comment

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

1 note. O/w LGTM.

camelcase: ['error', { properties: 'never' }],
camelcase: ['error', {
properties: 'never',
allow: ['UNSAFE_componentWillMount', 'UNSAFE_componentWillReceiveProps', 'UNSAFE_componentWillUpdate'],
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's definitely create a follow-up task to get rid of these UNSAFE_ usages and delete this configuration eventually.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Do you think it would be better to just copy this configuration over to the .eslintrc file in the website-django codebase and delete it once we fully deprecate those methods?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm okay either way. Since you're doing Team F work, feel free to create a card on our board as Cansin asked.

@@ -10,7 +10,10 @@ module.exports = {
// enforce one true brace style
Copy link
Contributor

Choose a reason for hiding this comment

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

Remember that it's really important to write your commit messages in the right format since they automatically control the versioning and the CHANGELOG. See the bottom of https://github.com/udemy/js-tooling. You may want to rebase and change how you worded your commit message.

Copy link
Contributor

@jjinux jjinux left a comment

Choose a reason for hiding this comment

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

Thanks!

camelcase: ['error', { properties: 'never' }],
camelcase: ['error', {
properties: 'never',
allow: ['UNSAFE_componentWillMount', 'UNSAFE_componentWillReceiveProps', 'UNSAFE_componentWillUpdate'],
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm okay either way. Since you're doing Team F work, feel free to create a card on our board as Cansin asked.

@kngroo
Copy link
Contributor Author

kngroo commented Dec 27, 2018

Closing this as per the discussion mentioned here: https://github.com/udemy/website-django/pull/31054#discussion_r244208432

Since this is a temporary rule, we want to override it in the .eslintrc file inside the website-django repo.

@kngroo kngroo closed this Dec 27, 2018
@kngroo kngroo deleted the improvement/allow-unsafe-camelcase branch December 27, 2018 21:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants