Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

fix(core): replace lodash/fp with lodash#718

Merged
miroslavstastny merged 12 commits intomasterfrom
fix/jest-leaks
Jan 19, 2019
Merged

fix(core): replace lodash/fp with lodash#718
miroslavstastny merged 12 commits intomasterfrom
fix/jest-leaks

Conversation

@miroslavstastny
Copy link
Member

@miroslavstastny miroslavstastny commented Jan 14, 2019

This PR is a workaround which fixes #703.
Instead of using lodash/fp, stardust now uses lodash.
This PR also runs jest tests with --detectLeaks as part of CI.

package.json Outdated
"tslint": "^5.11.0",
"tslint-config-airbnb": "^5.11.1",
"typescript": "~3.2.2",
"weak": "^1.0.1",
Copy link
Member Author

Choose a reason for hiding this comment

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

required by jest --detectLeaks

@miroslavstastny miroslavstastny changed the title fix(core): import lodash/fp functions one by one fix(core): replace lodash/fp with lodash Jan 16, 2019
@miroslavstastny miroslavstastny added 🚀 ready for review 🧰 fix Introduces fix for broken behavior. and removed 🚧 WIP labels Jan 16, 2019
@miroslavstastny miroslavstastny merged commit dfbfb58 into master Jan 19, 2019
@miroslavstastny miroslavstastny deleted the fix/jest-leaks branch January 19, 2019 20:01

return _.flow(
_.map((suggestion: string) => {
return _.chain(suggestions)
Copy link
Member

Choose a reason for hiding this comment

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

Heads up that _.chain causes all (or almost all) of lodash to be imported because all the chainable functions must be available on the returned object, and lodash can't statically detect which will be used (i.e. tree shaking will not work). Recommend refactoring to reduce the weight of our library.

https://medium.com/making-internets/why-using-chain-is-a-mistake-9bc1f80d51ba

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

Labels

🧰 fix Introduces fix for broken behavior. 🚀 ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

jest --detectLeaks reports memory leaks

3 participants