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 get method replaced with pure javascript #11916

Closed

Conversation

PunitSoniME
Copy link

Replace _.get with pure javascript code in order to reduce dependency on lodash

Addresses: #7747

Copy link
Contributor

@medikoo medikoo left a comment

Choose a reason for hiding this comment

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

@PunitSoniME great thanks for this work. Yet intention of #7747 was to replace _.get only in straightforward cases e.g. if (_.get(definitelyExistingObject, 'foo')) can be replaced with if (definitelyExistingObject.foo)

We should not introduce any _.get counterpart for that.

With next major, when we will drop support for Node.js v12, we would be able remove _.get completely by replacing it with Optional chaining

@GrahamCampbell
Copy link
Contributor

Could we could go ahead and remove node 12 support in a PR right now (I am happy to provide a first attempt at such a PR - though I am not familiar with all places in the codebase that are due to be refactored/cleaned up a little)? Node 12 has been EOL for some time, and GitHub Actions are running stuff that asks for node 12 on node 16 (https://github.blog/changelog/2023-05-04-github-actions-all-actions-will-run-on-node16-instead-of-node12/), effective May 18th.

@medikoo
Copy link
Contributor

medikoo commented May 15, 2023

Could we could go ahead and remove node 12 support in a PR right now

We will do that with the next major release (so v4), as removing support for any Node.js release (even EOL'd) is a breaking change

@GrahamCampbell
Copy link
Contributor

GrahamCampbell commented May 15, 2023

as removing support for any Node.js release (even EOL'd) is a breaking change

I don't agree here, actually. Correctly specified version constraints on the node version don't make changing supported versions breaking, since we can resolve the last version that supported node 12, automagically.

@medikoo
Copy link
Contributor

medikoo commented May 16, 2023

@GrahamCampbell it is breaking. In general, if something that worked before upgrading to a new release stopped working, it means there's a breaking change in the release
For users (or CIs) still working with Node.js v12, it'll be breaking to drop support for this version suddenly.

@PunitSoniME PunitSoniME closed this by deleting the head repository Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants