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 lint rule to check that memize() is used #6996

Merged
merged 1 commit into from May 30, 2018

Conversation

Projects
None yet
4 participants
@noisysocks
Member

noisysocks commented May 29, 2018

Fixes #6707.

We should be using memize over lodash's built-in memoize.

This adds a ESLint rule that warns us if import { memoize } from 'lodash' is used. It won't catch any usage that comes from import _ from 'lodash', but that's a rare case.

To test:

  1. Add import { memoize } from 'lodash'; to the top of any JavaScript file
  2. Run npm run lint
Add lint rule to check that memize() is used
We should be using memize instead of lodash's memoize function.

@noisysocks noisysocks requested a review from tofumatt May 29, 2018

@tofumatt

Looks good to me!

@noisysocks noisysocks merged commit 1b4898b into master May 30, 2018

2 checks passed

codecov/project 46.48% remains the same compared to d28fc33
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@noisysocks noisysocks deleted the add/eslint-rule-for-memize branch May 30, 2018

@aduth

This comment has been minimized.

Member

aduth commented May 30, 2018

It won't catch any usage that comes from import _ from 'lodash', but that's a rare case.

And a case we'd probably also want to prevent, in client-side code at least. While it's true we load the entire Lodash file anyways, I don't think we want to guarantee this to always be the case, if it proves in the future that cherry-picking individual functions can be possible / beneficial.

@mtias mtias added this to the 3.0 milestone Jun 4, 2018

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