chore(v2): replace few Lodash methods with native counterparts

[Simek]

Motivation

Same motivation like in previous PRs.

This is the initial step in getting rid of main lodash package in core v2 Docusaurus package. In this PR _.compact and _.flatten methods has been replaced with their native counterparts.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Same test results before and after code refactor.

Related PRs

• refactor(v2): use single method package instead of whole Lodash package in blog plugin #2460
• chore(v2): use single method Lodash packages in docs plugin #2519

[docusaurus-bot]

Deploy preview for docusaurus-2 ready!

Built with commit 2d9fb89

https://deploy-preview-2520--docusaurus-2.netlify.com

[yangshun]

I think I'd stick with the Lodash versions given they are more readable. Sorry about that.

[Simek]

@yangshun I think that it's not a good approach. This means that v2 core will have two more packages (lodash.compact and lodash.flatten) and two more dev packages (@types/lodash.compact and @types/lodash.flatten) just because of personal preferences.

I don't like either syntax and TS typing issues here personally, but in this case is a matter of preventing the bigger mess.

[yangshun]

@lex111 what do you think? Do you agree?

[Simek]

Also compact + flatten replacement could be extracted to the separate util/helper method if you think this improve readability, but I have no idea where this kind of code should be placed (in which package, which path).

[lex111]

I actually like what was done in this PR, and it seems to me that these changes do not complicate understanding of the code.

[Simek]

@lex111 Should I recreate this PR? I have also added one more minor commit to the changeset since this PR was closed which updates the pluginThemes to match other refactored cases (order of operations).

[lex111]

Let's wait for the final decision from @yangshun. пт, 3 апр. 2020 г. в 21:52, Bartosz Kaszubowski <notifications@github.com>:

…

@lex111 <https://github.com/lex111> Should I recreate this PR? I have also added one more minor commit to the changeset since this PR was closed which updates the pluginThemes to match other refactored cases (order of operations). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2520 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABBUIO26U4VOVMLGZD2ASCLRKYV5VANCNFSM4L4FYCBA> .

[Simek]

I have updated the reworked PR and removed also assign and isArray methods. All commits has been squashed. Current version can be seen here: Simek@2084e99

After applying those changes only has, isPlainObject and isString packages are needed in the core. I have a finishing commit stashed, which removes lodash dependency and adds listed above single method packages and typing for them.

Let me know if I should commit those changes in the separate PR after this part will be resolved or add them in reworked version of this PR.

[yangshun]

Ok let's do this. Could you open a new PR? I can't seem to reopen the PR now because it has been force pushed

[slorber]

Hey, was wondering what is the motivation to use lodash es exactly?

Personnally I find it highly annoying to have to update package.json everywhere I need a new lodash method.

What's the problem with a regular lodash available in node?

I understand we want to optimize client code, but don't really see the value for node code.

Btw, nothing will enforce the rule of "only lodash ES imports" over time. In fact, some code of mine was already merged and does not use lodash es but regular lodash, so wonder what to do about this?

If there are striong reasons to use lodash ES then we should enforce it with an ESLint import rule, but honnestly I'm not convinced by the value of enforcing such rule (at least for node code)

[slorber]

What I'd suggest is:

• prevent lodash import in client code by a nested .eslintrc in client code folders, with import rules preventing import from lodash(ideally, using vanilla js instead of lodash)
• allow using lodash in any server code, not necessarily from ES imports like this refactor introduced