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

chore(lodash): [RO-26515] Removes unneeded lodash deps with native replacements #105

Merged
merged 1 commit into from
Apr 9, 2024

Conversation

acolchado
Copy link
Collaborator

No description provided.

module.exports = function (Promise) {
class FixtureCache {

constructor() {
this.cache = {};

_bindAll(this, ['set', 'get', 'getOrPromise', 'setAsync']);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

getOrPromise is not called in an async method in the class, so context is not needed

module.exports = function (Promise, fsPromise, JSON) {
class Utils {

constructor() {
_bindAll(this, ['readFile', 'readJsonFile']);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not called within an async method, so removing.

module.exports = function (path, Promise, fsPromise, FixtureCache, Logger) {
class FixtureUtil {

constructor(fixturesPath) {
_bindAll(this, ['get', 'readAndProcessFile', 'startFileRead']);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not called within an async method, so removing.

@@ -1,9 +1,7 @@
const _compact = require('lodash.compact');
const _map = require('lodash.map');
const _invokeMap = require('lodash.invokemap');
Copy link
Contributor

@otrreeves otrreeves Apr 9, 2024

Choose a reason for hiding this comment

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

I believe the _invokeMap usage in this file could be replaced with

return self._pluginInstances.map(plugin => plugin.end());

Not sure if this is the only usage of _invokeMap though

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I believe so but it's not low-hanging fruit. I will take a look at it in the next set of PRs. Coverage is not great to catch is so I don't want to commit to writing additional tests and delaying the main need of the project right now.

@acolchado acolchado merged commit 221bf1b into main Apr 9, 2024
2 checks passed
@acolchado acolchado deleted the AC/lodash-cleanup branch April 9, 2024 01:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants