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

Replace per-method packages with the main lodash package. #4005

Merged
merged 8 commits into from
Jan 26, 2024

Conversation

garg3133
Copy link
Member

As this page states, use of the per-method lodash packages is discouraged: https://lodash.com/per-method-packages and these packages are no longer updated. So, we should replace all the per-method lodash packages with just the main lodash package.

This PR does so for the lodash.pick package only for now to fix the security vulnerability in that package, so that we can put the fix in the next planned Nightwatch release.

For the other similar per-method packages, we can replace them with lodash afterwards, as these upgrades may require some more attention to ensure we don't end up breaking anything.

Fixes: #4003

Copy link

Status

  • ❌ No modified files found in the types directory.
    Please make sure to include types for any changes you have made. Thank you!.

@garg3133
Copy link
Member Author

Opened #4006 to track updates for other per-method packages.

@beatfactor
Copy link
Member

@garg3133 isn't it possible to just upgrade the lodash.pick? if we opt for adding the entire lodash as a dependency than we should also remove the other lodash.something packages and have just the main one.

@garg3133
Copy link
Member Author

@beatfactor lodash.pick is no longer updated (it was last updated 7 years ago, while lodash was last updated 3 years ago).

Okay, I'll try to replace other per-method packages as well.

@garg3133
Copy link
Member Author

garg3133 commented Jan 25, 2024

@beatfactor I had to revert lodash.clone change because since that commit most of our tests in CI are timing out across all the platforms and node versions, and even after 20 mins the CI is still running.

(Tests are passing within usual time after reverting that change.)

@garg3133 garg3133 changed the title Replace lodash.pick with main lodash package. Replace per-method packages with the main lodash package. Jan 25, 2024
@garg3133
Copy link
Member Author

The issue with lodash.clone seems to be that we are currently using the v3 of lodash.clone but replacing it with the lodash package will result in us using the v4, which might be having some breaking changes and hence the tests are timing out/failing.

Even if we upgrade lodash.clone to v4 instead of replacing it with lodash directly, tests still fail. So, we'd need to first migrate our usage of lodash.clone to v4 and then replace it with lodash.

@garg3133
Copy link
Member Author

garg3133 commented Jan 25, 2024

Starting with v4, clone can no longer be used for deep copy, so had to use cloneDeep instead.

@garg3133 garg3133 linked an issue Jan 25, 2024 that may be closed by this pull request
@beatfactor beatfactor merged commit 6ffc3b6 into nightwatchjs:main Jan 26, 2024
17 checks passed
@garg3133 garg3133 deleted the lodash.pick branch January 26, 2024 09:21
@BeniRupp
Copy link

BeniRupp commented Mar 5, 2024

Can we backport this to version 2.6?

That would be very helpful to get rid of the vulnerability in lodash.pick.

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.

Replace per-method lodash packages with the main lodash package. GHSA-p6mc-m468-83gw
3 participants