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

Testing: Enforce array as Lodash path argument #6247

Merged
merged 2 commits into from
May 1, 2018

Conversation

aduth
Copy link
Member

@aduth aduth commented Apr 18, 2018

This pull request seeks to enforce a new custom ESLint rule encouraging the use of an array as the second parameter of Lodash functions accepting a "path" (e.g. _.get). The intent here is to skip a parsing step to convert a string path to its array equivalent, which occurs internally in Lodash (source).

The performance impact is not as bad as I had suspected for simple strings, as the internal logic tests for properties, skipping the parse for use like _.get( foo, 'bar' ). Further, its stringToPath is memoized so only calculated once. All the same, this rule encourages consistency, avoids any such regular expression testing, avoids memory overhead of memoization, and avoids the string ever needing to be parsed.

Testing instructions:

There should be no behavior impact of these changes.

Ensure tests pass:

npm test

@aduth aduth added [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. [Type] Performance Related to performance efforts labels Apr 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. [Type] Performance Related to performance efforts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant