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

Code Cleanup: Enforce Linting Rules & Update UNSAFE_ React Lifecycle Events #559

Merged
merged 33 commits into from Jun 4, 2020
Merged
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
75b0091
add linting for no-param-reassign and fix resulting linting errors
IAmThePan Apr 29, 2020
8d37082
add linting for prefer-object-spread and fix resulting linting errors
IAmThePan Apr 29, 2020
3049bd1
add linting for no-restricted-syntax and fix 1/2 of resulting errors
IAmThePan May 1, 2020
fe03ad7
add linting for no-prototype-builtins and fix resulting linting errors
IAmThePan May 4, 2020
f11bc18
add linting for class-methods-use-this and fix most resulting errors.…
IAmThePan May 5, 2020
5f3471e
finish linting for class-methods-use-this
IAmThePan May 6, 2020
d4d84dc
add linting for no-mixed-operators and fix resulting linting errors
IAmThePan May 6, 2020
17f90ed
add linting for import/prefer-default-export and fix resulting lintin…
IAmThePan May 8, 2020
a77cd1b
add linting for react/no-access-state-in-setstate and fix resulting l…
IAmThePan May 8, 2020
c31dbaf
add linting for react/jsx-props-no-spreading and fix resulting lintin…
IAmThePan May 11, 2020
3d50aff
finish linting errors for no-restricted-syntax. 1 remains: couldn't r…
IAmThePan May 12, 2020
227d012
Merge branch 'develop' into feature/update-linter
IAmThePan May 12, 2020
0988a29
Fix linting errors resulting from the merge with develop
IAmThePan May 12, 2020
b55be21
Refactor UNSAFE_componentWillMount into either constructor or compone…
IAmThePan May 14, 2020
241747c
Refactor UNSAFE_componentWillReceiveProps to componentDidUpdate or ge…
IAmThePan May 18, 2020
eb413fb
re-enable lint exception for no-prototype-builtins and revert calls b…
IAmThePan May 20, 2020
ac0ec93
add single line exception for no-restricted-syntax linting rule
IAmThePan May 20, 2020
fd954eb
add linting for react/destructuring-assignment and fix errors. ToDo: …
IAmThePan May 21, 2020
e815546
Fix minor bugs
IAmThePan May 22, 2020
ae8ffcc
Fix General Settings last updated text
IAmThePan May 22, 2020
4c56460
rework linting rule no-param-reassign to have more exceptions and par…
IAmThePan May 23, 2020
c5ef663
Remove file and line linting exceptions.
IAmThePan May 26, 2020
3ca2402
re-add linting rule react/sort-comp and fix resulting errors
IAmThePan May 26, 2020
4533fd7
remove added linting exception consistent-return and fix resulting er…
IAmThePan May 26, 2020
590aa60
remove added linting expression no-use-before-define and fix resultin…
IAmThePan May 26, 2020
ecb62e8
Fix linting error
IAmThePan May 26, 2020
f634c1d
fix minor bugs
IAmThePan May 26, 2020
ea51432
Code cleanup: fix PromoModal imports
IAmThePan May 26, 2020
75a88cb
Merge with develop. Fix resulting linting errors
IAmThePan May 26, 2020
68493d2
remove unnecessary hasOwnProperty calls after refactored for...in loops
IAmThePan May 28, 2020
6297062
Fix missing strings bug
IAmThePan May 28, 2020
9b4d16c
Fix last remaining string bug
IAmThePan May 28, 2020
2cb47a1
Merge branch 'develop' into feature/cleanup
IAmThePan Jun 4, 2020
File filter
Filter file types
Jump to
Jump to file
Failed to load files.

Always

Just for now

@@ -40,7 +40,7 @@ module.exports = {
rules: {
'arrow-parens': [2, 'as-needed', { 'requireForBlockBody': true }],
'camelcase': [0],
'class-methods-use-this': [0],
'class-methods-use-this': [1],
This conversation was marked as resolved by wlycdgr

This comment has been minimized.

@christophertino

christophertino Jun 4, 2020
Member

We can remove this line entirely and let the AirBnB defaults pick it up.

This comment has been minimized.

@IAmThePan

IAmThePan Jun 11, 2020
Author Contributor

updated in feature/cleanup-cleanup as this PR was already merged.

This comment has been minimized.

@wlycdgr

wlycdgr Jun 11, 2020
Member

lgtm in feature/cleanup-cleanup

'comma-dangle': [2, {
'arrays': 'only-multiline',
'objects': 'only-multiline',
@@ -54,37 +54,50 @@ module.exports = {
'lines-between-class-members': [1],
'max-len': [0],
'newline-per-chained-call': [0, { 'ignoreChainWithDepth': 2 }],
'no-mixed-operators': [0],
'no-mixed-operators': [1],
This conversation was marked as resolved by wlycdgr

This comment has been minimized.

@christophertino

christophertino Jun 4, 2020
Member

Same, remove this line.

This comment has been minimized.

@IAmThePan

IAmThePan Jun 11, 2020
Author Contributor

updated in feature/cleanup-cleanup as this PR was already merged.

This comment has been minimized.

@wlycdgr

wlycdgr Jun 11, 2020
Member

lgtm in feature/cleanup-cleanup

'no-nested-ternary': [0],
'no-param-reassign': [0], // TODO: enable this check
'no-param-reassign': ['error', {
props: true,
ignorePropertyModificationsFor: [
'acc', // for reduce accumulators
'document', // for IIFE with window & document params
'c2pFrame', // for mutable iframe c2p element
'trackerEl', // for trackers.forEach()
'categoryEl', // for categories.forEach()
'dataEntry', // for data.forEach()
'appEntry', // for apps.forEach()
'sEntry', // for apps.forEach()
'confMutable', // for mutable conf settings
'eventMutable', // for mutable event details objects
]
}],
'no-plusplus': [0],
'no-prototype-builtins': [0], // TODO: enable this check
'no-restricted-syntax': [0], // TODO: enable this check
'no-prototype-builtins': [0],
'no-restricted-syntax': [1],
This conversation was marked as resolved by wlycdgr

This comment has been minimized.

@christophertino

christophertino Jun 4, 2020
Member

Remove 'no-restricted-syntax'

This comment has been minimized.

@IAmThePan

IAmThePan Jun 11, 2020
Author Contributor

updated in feature/cleanup-cleanup as this PR was already merged.

This comment has been minimized.

@wlycdgr

wlycdgr Jun 11, 2020
Member

lgtm in feature/cleanup-cleanup

'no-tabs': [0],
'no-underscore-dangle': [0],
'no-unused-vars': [1],
'no-useless-escape': [1],
'operator-linebreak': [0],
'prefer-object-spread': [0], // TODO: enable this check
'prefer-object-spread': [1],
This conversation was marked as resolved by wlycdgr

This comment has been minimized.

This comment has been minimized.

@IAmThePan

IAmThePan Jun 11, 2020
Author Contributor

updated in feature/cleanup-cleanup as this PR was already merged.

This comment has been minimized.

@wlycdgr

wlycdgr Jun 11, 2020
Member

lgtm in feature/cleanup-cleanup

'space-before-function-paren': [2, 'never'],
'template-curly-spacing': [0],

// Plugin: Import
'import/no-cycle': [0],
'import/prefer-default-export': [0],
'import/prefer-default-export': [1],
This conversation was marked as resolved by wlycdgr

This comment has been minimized.

This comment has been minimized.

@IAmThePan

IAmThePan Jun 11, 2020
Author Contributor

updated in feature/cleanup-cleanup as this PR was already merged.

This comment has been minimized.

@wlycdgr

wlycdgr Jun 11, 2020
Member

lgtm in feature/cleanup-cleanup


// Plugin: React
'react/destructuring-assignment': [0],
'react/destructuring-assignment': [1],
This conversation was marked as resolved by wlycdgr

This comment has been minimized.

This comment has been minimized.

@IAmThePan

IAmThePan Jun 11, 2020
Author Contributor

updated in feature/cleanup-cleanup as this PR was already merged.

This comment has been minimized.

@wlycdgr

wlycdgr Jun 11, 2020
Member

lgtm in feature/cleanup-cleanup

'react/static-property-placement': [0],
'react/jsx-curly-newline': [0],
'react/jsx-indent': [1, 'tab'],
'react/jsx-indent-props': [1, 'tab'],
'react/jsx-props-no-spreading': [0], // TODO: enable this check
'react/no-access-state-in-setstate': [0], // TODO: enable this check
'react/jsx-props-no-spreading': [1],
'react/no-access-state-in-setstate': [1],
This conversation was marked as resolved by wlycdgr

This comment has been minimized.

@christophertino

christophertino Jun 4, 2020
Member

Remove both jsx-props-no-spreading and no-access-state-in-setstate

This comment has been minimized.

@IAmThePan

IAmThePan Jun 11, 2020
Author Contributor

updated in feature/cleanup-cleanup as this PR was already merged.

This comment has been minimized.

@wlycdgr

wlycdgr Jun 11, 2020
Member

lgtm in feature/cleanup-cleanup

'react/no-danger': [0],
'react/prop-types': [0],
'react/jsx-fragments': [1, 'element'],
'react/sort-comp': [2, {
'react/sort-comp': [1, {
order: [
"static-variables",
"instance-variables",
@@ -104,5 +117,8 @@ module.exports = {
'jsx-a11y/mouse-events-have-key-events': [0],
'jsx-a11y/no-noninteractive-element-interactions': [0],
'jsx-a11y/no-static-element-interactions': [0],
'jsx-a11y/label-has-associated-control': [0], //ToDo: enable this check. Throws 2 errors, should work.
'jsx-a11y/media-has-caption': [0], //ToDo: enable this check. Need Captions to enable.
'jsx-a11y/anchor-has-content': [0], //ToDo: enable this check. Need anchor text to enable.
},
};
@@ -704,6 +704,9 @@
"panel_detail_menu_premium_title": {
"message": "Premium"
},
"panel_detail_learn_more": {
"message": "Learn More"
},
"panel_detail_history_title": {
"message": "History"
},
@@ -39,61 +39,62 @@ export default (state = initialState, action) => {
case UPDATE_PANEL_DATA: {
const { account } = action.data;
if (account === null) {
return Object.assign({}, initialState);
return { ...initialState };
}
const {
userID, user, userSettings, subscriptionData
} = account;
return Object.assign({}, state, {
return {
...state,
loggedIn: true,
userID,
user,
userSettings,
subscriptionData,
});
subscriptionData
};
}
case REGISTER_SUCCESS:
case LOGIN_SUCCESS: {
return Object.assign({}, state, {
loggedIn: true,
});
return { ...state, loggedIn: true };
}
case LOGOUT_SUCCESS: {
return Object.assign({}, initialState);
return { ...initialState };
}
case GET_USER_SUCCESS: {
const { user } = action.payload;
return Object.assign({}, state, {
return {
...state,
loggedIn: true,
user
});
};
}
case GET_USER_SETTINGS_SUCCESS: {
const { settings } = action.payload;
return Object.assign({}, state, {
return {
...state,
loggedIn: true,
userSettings: settings
});
};
}
case GET_USER_SUBSCRIPTION_DATA_FAIL: {
const { subscriptionData } = initialState;
return Object.assign({}, state, {
subscriptionData,
});
return { ...state, subscriptionData };
}
case GET_USER_SUBSCRIPTION_DATA_SUCCESS: {
const { subscriptionData } = action.payload;
return Object.assign({}, state, {
return {
...state,
loggedIn: true,
subscriptionData
});
};
}
case RESET_PASSWORD_SUCCESS: {
const toastMessage = t('banner_check_your_email_title');
return Object.assign({}, state, {
return {
...state,
toastMessage,
resetPasswordError: false
});
};
}
case RESET_PASSWORD_FAIL: {
const { errors } = action.payload;
@@ -108,10 +109,11 @@ export default (state = initialState, action) => {
errorText = t('server_error_message');
}
});
return Object.assign({}, state, {
return {
...state,
toastMessage: errorText,
resetPasswordError: true
});
};
}

default: return state;
@@ -160,11 +160,11 @@ const Click2PlayContentScript = (function(win, doc) {
if (name === 'c2p') {
if (message) {
// Dequeue C2P data stored while the script injection was taking place
for (const app_id in message) {
if (message.hasOwnProperty(app_id)) {
applyC2P(app_id, message[app_id].data, message[app_id].html);
delete message[app_id];
}
const messageKeys = Object.keys(message);
for (let i = 0; i < messageKeys.length; i++) {
const app_id = messageKeys[i];
applyC2P(app_id, message[app_id].data, message[app_id].html);
delete message[app_id];
}
}
}
@@ -27,7 +27,8 @@ const { sendMessage } = msg;
* Use to call init to initialize functionality
* @var {Object} initialized to an object with init method as its property
*/
const PageInfo = (function(window, document) {
const PageInfo = (function(window, doc) {
const document = doc;
let state = document.readyState;
/**
* Calculate page domain and latency. Send pageInfo to background.js.
@@ -13,8 +13,6 @@
/**
* @namespace ContentScriptUtils
*/
/* eslint no-use-before-define: 0 */

import { log } from '../../../src/utils/common';
import { sendMessageInPromise as panelSendPromiseMessage, sendMessage as panelSendMessage } from '../../panel/utils/msg';

@@ -31,7 +31,8 @@ class AppView extends Component {
* Lifecycle Event
*/
componentDidUpdate(prevProps) {
if (prevProps.children !== this.props.children) {
const { children } = this.props;
if (prevProps.children !== children) {
this.mainContent.current.scrollTop = 0;
}
}
@@ -25,7 +25,8 @@ class AppViewContainer extends Component {
* Handle clicking to exit the Toast Message.
*/
_exitToast = () => {
this.props.actions.setToast({
const { actions } = this.props;
actions.setToast({
toastMessage: '',
toastClass: '',
});
@@ -36,11 +37,13 @@ class AppViewContainer extends Component {
* @return {JSX} JSX for rendering the Home View of the Hub app
*/
render() {
const childProps = {
...this.props,
exitToast: this._exitToast,
};
return <AppView {...childProps} />;
const { app, children } = this.props;

return (
<AppView app={app} exitToast={this._exitToast}>
{children}
</AppView>
);
}
}

@@ -19,12 +19,13 @@ function AppViewReducer(state = initialState, action) {
switch (action.type) {
case SET_TOAST: {
const { toastMessage, toastClass } = action.data;
return Object.assign({}, state, {
app: Object.assign({}, {
return {
...state,
app: {
toastMessage,
toastClass
}),
});
}
};
}
default: return state;
}
@@ -23,7 +23,7 @@ import AppViewReducer from './AppViewReducer';
* @return {function} this function returns a plain object, which will be merged into the component's props
* @memberof HubContainers
*/
const mapStateToProps = state => Object.assign({}, state.app);
const mapStateToProps = state => ({ ...state.app });

/**
* Bind the component's action creators using Redux's bindActionCreators.
@@ -32,7 +32,7 @@ const mapStateToProps = state => Object.assign({}, state.app);
* @memberof SetupContainers
*/
const mapDispatchToProps = dispatch => ({
actions: bindActionCreators(Object.assign({}, { setToast }), dispatch),
actions: bindActionCreators({ setToast }, dispatch),
});

export const reducer = AppViewReducer;
@@ -105,7 +105,7 @@ const CreateAccountView = (props) => {
/>
{confirmEmailError && (
<div className="CreateAccountView__inputError">
{t('your_emails_do_not_match')}
{t('your_email_do_not_match')}
This conversation was marked as resolved by christophertino

This comment has been minimized.

@christophertino

christophertino Jun 4, 2020
Member

We probably should fix this typo in the key name, but would require fixing all the languages in Transifex. Leave it for now and I'll make a note to fix it later.

This comment has been minimized.

@IAmThePan

IAmThePan Jun 11, 2020
Author Contributor

I changed the key to match the messages.json file.
Do you want me to change all the messages.json files?

This comment has been minimized.

@wlycdgr

wlycdgr Jun 15, 2020
Member

@christophertino sounds like you think we should punt on this?

</div>
)}
</div>
ProTip! Use n and p to navigate between commits in a pull request.