Import per-function lodash modules

[jayaddison]

Q	A
Fixed Issues?	Fixes #11726
Patch: Bug Fix?	No
Major: Breaking Change?	No
Minor: New Feature?	No
Tests Added + Pass?	No
Any Dependency Changes?	Yes
License	MIT

Reading some comments on #11726, it sounds like it should be possible to replace the full lodash dependency included in many of the babel packages with smaller, per-function lodash function modules.

To explain that differently: where code previously used import clone from "lodash/clone" from the full lodash package, instead it should be possible to import clone from "lodash.clone" using the smaller lodash.clone package.

There are some concerns and gotchas here:

• ⚠️ Package versions for the per-function lodash modules vary, and have not yet been analyzed for feature-parity against the current lodash release (4.17.15)
• ℹ️ The lodash.extends package does not exist, but lodash.assignIn is an alias for lodash.extends so we can use the corresponding package
• ℹ️ It was necessary to revert Add @babel/eslint-plugin-development-internal #11376 to build and test this locally, and local test results still look patchy at the time of writing

Any ideas for checking the freshness and correctness of the per-function packages would be appreciated; performing that verification should be considered a blocker for this approach.

[babel-bot]

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/32404/

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 35a0768:

Sandbox	Source
babel-repl-custom-plugin	Configuration
babel-plugin-multi-config	Configuration

[JLHwung]

I suggest we replace some of lodash usage by builtin functions, which should fall into a separate PR. After that PR is merged, we can replaced the more-than-trivial lodash usage by per-function modules.

https://github.com/you-dont-need/You-Dont-Need-Lodash-Underscore may be helpful.

[jayaddison]

I suggest we replace some of lodash usage by builtin functions, which should fall into a separate PR. After that PR is merged, we can replaced the more-than-trivial lodash usage by per-function modules.

That sounds like a good approach to me. I might have time to try that over the weekend; either way, anyone should feel free to create the builtin-replacement-PR and I'll merge main back in here and resolve (expected) conflicts after it's merged.

[ark120202]

Note that per-module lodash packages apparently won't be supported in the future, see lodash/lodash#3838

[jayaddison]

Ah, thanks @ark120202. That could pour a bit of cold water on this plan.

[jayaddison]

lodash.has is similar to lodash.get, and lodash.get appears in You-Dont-Need-Lodash-Underscore.

That said, the YDNLU native JavaScript replacement is relatively complex (9 lines of non-trivial code including regular expressions) and generates an error when key paths do not exist. On balance it may make sense to maintain readability for now, although that's just my two cents and I'd welcome feedback.

[nicolo-ribaudo]

Let's keep it, since we are using it with a deep path.

[merceyz]

As @ark120202 mentioned the per method packages are deprecated and aren't updated anymore, https://lodash.com/per-method-packages, using them also results in larger installs as it's most likely some other dependency has lodash as one of its dependencies, meaning the main lodash package and the per method packages will be installed

[existentialism]

We should be able to remove defaults, yeah?

const res = await util.transform(
      cliOptions.filename,
      code,
      {
        ...babelOptions,
        sourceFileName: "stdin",
      },
);

[jayaddison]

Possibly! - we'd want to be careful about the argument ordering to the spread operator:

> var defaults = {'option': 'valueFromDefault'};
> var cliArgs = {'option': 'commandLineArgument'};
> {...defaults, ...cliArgs}
{ option: 'commandLineArgument' }
> {...cliArgs, ...defaults}
{ option: 'valueFromDefault' }

Also a subtle difference is that lodash.defaults mutates the first argument and returns it as a value, whereas the spread operator returns a new shallow clone; that's probably fine and safe in most cases, but worth being careful about.

[nicolo-ribaudo]

In our case the mutation isn't a problem, since the first object is always defined inline in the call.

[jayaddison]

Okie doke, thanks - #11797 contains an attempt to clean up the use of lodash.defaults from the codebase (including this callsite).

[jayaddison]

It might be possible to replace lodash/escapeRegExp with a function provided by the MDN docs, although I don't know whether it'd make sense to replicate that per-babel-package or whether it should go into a utility/common package if so.

[coreyfarrell]

Could we instead consider replacing it with escape-string-regexp. That module is well tested, not being deprecated and gets 19 million weekly downloads (it's not below scrutiny).

[TrySound]

Keep in mind it requires major bump
https://github.com/sindresorhus/escape-string-regexp/blob/master/.travis.yml

[coreyfarrell]

Ah sorry for missing that, I guess this suggestion would only apply to babel 8 then.

[TrySound]

lodash/cloneDeep can be replaced with great klona. Node v8 is required, will be a breaking change.

[TrySound]

lodash/has perhaps can be replaced with dlv which is more like lodash/get. Node v6 is supported.

[TrySound]

is-plain-object is very small replacement for lodash/isPlainObject. Supports node 0.10.

[jayaddison]

lodash/cloneDeep can be replaced with great klona. Node v8 is required, will be a breaking change.

Thanks @TrySound - do you have a sense for how klona compares to the widely-deployed clone library?

A few notes on lukeed/klona#13 make it sound like there's functionality yet to be implemented, and clone seems to provide support for some of those via API options.

[jayaddison]

lodash/has perhaps can be replaced with dlv which is more like lodash/get. Node v6 is supported.

Wow, dlv has an impressive commitment towards dependency size reduction :)

I don't know what to suggest here. There's certainly an argument for keeping the bundled dependencies small, but I also think there's a strong safety argument for using libraries that are stable and avoid JavaScript tricks that could potentially lead to unexpected gotchas/situations in future (in particular the use of an undef argument seems like a neat size-reduction measure, but potentially problematic in future).

The lowest-risk option might be not to change the existing implementation (i.e. continue to use lodash/has) until there's a compelling reason to (and yep, I realize that'd block the goal of removing lodash completely, but it'd be better to maintain software safety and quality rather than risk user impact).

[jayaddison]

I think I'm going to close this and a few of the related lodash-replacement pull requests since the original concerns regarding lodash maintenance seem to have subsided (at least for now), and so the PRs just exist as extra overhead for everyone.

Generally I'd delete the branches and then also the source repository once done; if I remember this makes it tricky to retrieve the commits later, so.. does anyone have opinions about whether that's reasonable to do? (I don't think it'd be super challenging for someone to rewrite these if needed -- and as far as I know GitHub will still show the PR diff so it'd be possible to recreate them that way too).

[yumetodo]

@jayaddison AFAIK, After you delete the jayaddison:dependencies/lodash-submodules, the pull/11789/head branch will still exist.

[jayaddison]

Thanks @yumetodo - let's hope that should be the case; I seem to remember running into problems even with the pr refs previously, but forget exactly what happened. Probably user error :)

[jayaddison]

All good: here's how I confused myself previously: getpelican/pelican-themes#659 (comment) (it was possible to recover by opening a new pull request)

[nicolo-ribaudo]

Hey thanks for closing these PRs: we usually keep them open for years until someone from the team finally reviews and merges them, but since this is not necessary anymore (we might still remove lodash in the future, but it's not urgent) I appreciate that you avoided keeping them around forever 😅