Adding comment about importing lodash library #78156

brittanyjoiner15 · 2020-09-22T14:10:59Z

Was initially working on #74539 to clean up lodash imports and pull the specific module rather than entire lodash library, but @spalger has a better improvement in #78100 that would clean up bundling by importing the entire library.

This PR has found instances across Kibana where people are referencing lodash and not adding the entire library, but should be per Spencer's new update.

spalger · 2020-09-22T15:17:42Z

x-pack/plugins/actions/server/builtin_action_types/resilient/service.ts

@@ -5,6 +5,8 @@
 */

 import axios from 'axios';
+// Prefer  importing entire lodash library, e.g. import { get } from "lodash"
+// eslint-disable-next-line no-restricted-imports
 import { omitBy, isNil } from 'lodash/fp';


I think we need an exception for lodash/fp as I'm pretty sure you can't access lodash/fp any other way. I've added that import to the shared bundle so we should allow people to use it.

Ah i briefly thought about this and then forgot when I did the find and replace.

@smith can you help me with what the regex would be for this?
find // Prefer importing entire lodash library, e.g. import { get } from "lodash"\n// es-lint-disable-next-line no-restricted-imports\nimport(.*)from 'lodash/fp';
replace import$1from 'lodash/fp';

?

@spalger just updated the rule and removed comments on lodash/fp imports!

spalger

LGTM!

elasticmachine · 2020-09-22T20:23:38Z

Pinging @elastic/apm-ui (Team:apm)

cjcenizal

ES UI comments LGTM.

Dosant · 2020-09-23T08:32:20Z

@brittanyjoiner15, @spalger want to confirm: because of #78100 importing lodash/* instead of lodash would bundle redundant code, correct?

peteharverson

A lot of the ml files, plus a few others, have had extra blank lines added without comments. Is that a mistake?

x-pack/plugins/ml/server/models/annotation_service/annotation.ts

x-pack/plugins/ml/server/models/bucket_span_estimator/bucket_span_estimator.js

x-pack/plugins/ml/server/models/data_visualizer/data_visualizer.ts

x-pack/plugins/ml/server/models/results_service/build_anomaly_table_items.js

x-pack/plugins/security_solution/server/utils/runtime_types.ts

ogupte

APM comments looking good to me!

weltenwort

infra plugin changes LGTM

peteharverson

ML and Transforms plugin changes LGTM

spong

Security changes LGTM! 👍

kibanamachine · 2020-09-24T16:37:50Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 22c3ed6

Metrics [docs]

✅ unchanged

History

💚 Build #76783 succeeded a807767
💚 Build #76355 succeeded 63d3d62
💔 Build #76312 failed bb341db
💔 Build #76195 failed d4bd1f3
💔 Build #76190 failed fdf4efb

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

kibanamachine · 2020-09-28T16:38:45Z

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 78156 or prevent reminders by adding the backport:skip label.

* added comment about importing lodash library * fixed space with prefer * cleaned up extra space and removed comments for lodash/fp * took out the comment in server files * Remove newlines Co-authored-by: Nathan L Smith <[email protected]>

brittanyjoiner15 · 2020-09-28T19:12:54Z

@smith I just did the backport for 7.x

kibanamachine · 2020-09-29T16:48:44Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

kibanamachine · 2020-09-30T16:54:38Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

kibanamachine · 2020-10-01T17:04:41Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

* Adding comment about importing lodash library (#78156)

added comment about importing lodash library

fdf4efb

brittanyjoiner15 requested review from smith and spalger September 22, 2020 14:10

brittanyjoiner15 self-assigned this Sep 22, 2020

fixed space with prefer

d4bd1f3

spalger reviewed Sep 22, 2020

View reviewed changes

cleaned up extra space and removed comments for lodash/fp

bb341db

spalger approved these changes Sep 22, 2020

View reviewed changes

brittanyjoiner15 marked this pull request as ready for review September 22, 2020 20:17

brittanyjoiner15 requested review from a team as code owners September 22, 2020 20:17

brittanyjoiner15 requested a review from a team September 22, 2020 20:17

scottybollinger approved these changes Sep 22, 2020

View reviewed changes

botelastic bot added the Team:APM All issues that need APM UI Team support label Sep 22, 2020

cjcenizal approved these changes Sep 22, 2020

View reviewed changes

smith added v7.10.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes labels Sep 22, 2020

took out the comment in server files

63d3d62

peteharverson reviewed Sep 23, 2020

View reviewed changes

Remove newlines

a807767

ogupte approved these changes Sep 24, 2020

View reviewed changes

mshustov approved these changes Sep 24, 2020

View reviewed changes

weltenwort approved these changes Sep 24, 2020

View reviewed changes

peteharverson approved these changes Sep 24, 2020

View reviewed changes

spong approved these changes Sep 24, 2020

View reviewed changes

smith removed their request for review September 24, 2020 14:28

smith mentioned this pull request Sep 24, 2020

changing lodash #74539

Closed

Merge remote-tracking branch 'upstream/master' into bjoiner/lodashClean

22c3ed6

brittanyjoiner15 merged commit 63ce752 into elastic:master Sep 24, 2020

mshustov mentioned this pull request Sep 24, 2020

fix lodash imports #78456

Merged

walterra mentioned this pull request Sep 28, 2020

[ML] Limit exposing shared static code through ml/public/index.ts. #77745

Merged

2 tasks

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Sep 28, 2020

smith mentioned this pull request Sep 28, 2020

[7.x] Adding comment about importing lodash library (#78156) #78646

Closed

smith mentioned this pull request Sep 28, 2020

[APM] Use default imports from lodash #73937

Closed

brittanyjoiner15 mentioned this pull request Sep 28, 2020

[7.x] Adding comment about importing lodash library (#78156) #78667

Merged

Dosant mentioned this pull request Sep 30, 2020

Drilldowns in examples #75640

Merged

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Oct 1, 2020

smith pushed a commit that referenced this pull request Oct 1, 2020

[7.x] Adding comment about importing lodash library (#78156) (#78667)

ca42caa

* Adding comment about importing lodash library (#78156)

spong mentioned this pull request Oct 6, 2020

[Security Solution][Detections] Convert EQL validation to use search strategy #79538

Merged

2 tasks

jloleysens mentioned this pull request Nov 18, 2020

[ILM] Policy form should not throw away data #83077

Merged

1 task

nickpeihl mentioned this pull request Nov 15, 2022

[controls] fix timeslider control not filtering dashboard panels #145184

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding comment about importing lodash library #78156

Adding comment about importing lodash library #78156

brittanyjoiner15 commented Sep 22, 2020

spalger Sep 22, 2020

brittanyjoiner15 Sep 22, 2020 •

edited

Loading

brittanyjoiner15 Sep 22, 2020

spalger left a comment

elasticmachine commented Sep 22, 2020

cjcenizal left a comment

Dosant commented Sep 23, 2020

peteharverson left a comment

ogupte left a comment

weltenwort left a comment

peteharverson left a comment

spong left a comment

kibanamachine commented Sep 24, 2020

kibanamachine commented Sep 28, 2020

brittanyjoiner15 commented Sep 28, 2020 •

edited

Loading

kibanamachine commented Sep 29, 2020

kibanamachine commented Sep 30, 2020

kibanamachine commented Oct 1, 2020

Adding comment about importing lodash library #78156

Adding comment about importing lodash library #78156

Conversation

brittanyjoiner15 commented Sep 22, 2020

spalger Sep 22, 2020

Choose a reason for hiding this comment

brittanyjoiner15 Sep 22, 2020 • edited Loading

Choose a reason for hiding this comment

brittanyjoiner15 Sep 22, 2020

Choose a reason for hiding this comment

spalger left a comment

Choose a reason for hiding this comment

elasticmachine commented Sep 22, 2020

cjcenizal left a comment

Choose a reason for hiding this comment

Dosant commented Sep 23, 2020

peteharverson left a comment

Choose a reason for hiding this comment

ogupte left a comment

Choose a reason for hiding this comment

weltenwort left a comment

Choose a reason for hiding this comment

peteharverson left a comment

Choose a reason for hiding this comment

spong left a comment

Choose a reason for hiding this comment

kibanamachine commented Sep 24, 2020

💚 Build Succeeded

Metrics [docs]

History

kibanamachine commented Sep 28, 2020

brittanyjoiner15 commented Sep 28, 2020 • edited Loading

kibanamachine commented Sep 29, 2020

kibanamachine commented Sep 30, 2020

kibanamachine commented Oct 1, 2020

brittanyjoiner15 Sep 22, 2020 •

edited

Loading

brittanyjoiner15 commented Sep 28, 2020 •

edited

Loading