[ML] Removing full lodash library imports by jgowdyelastic · Pull Request #74742 · elastic/kibana

jgowdyelastic · 2020-08-11T09:58:24Z

Removes all import _ from 'lodash' and replaces them with imports for each lodash function used.
All uses of _.has, _.first have been removed.
Most uses of _.last have been removed
All uses of _.get, _.each, _.filter _.reduce and _.find should also be removed, but can be done in a follow up PR.

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2020-08-11T11:11:10Z

Pinging @elastic/ml-ui (:ml)

alvarezmelissa87 · 2020-08-11T15:21:28Z

x-pack/plugins/ml/public/application/services/results_service/result_service_rx.ts

-import _ from 'lodash';
+import each from 'lodash/each';
+import get from 'lodash/get';
+// import map from 'lodash/map';


Is this comment needed?

alvarezmelissa87

Tested and LGTM ⚡

kibanamachine · 2020-08-11T19:17:18Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: e5e8d5d

Build metrics

@kbn/optimizer bundle module count

id	value	diff	baseline
ml	1353	+236	1117

async chunks size

id	value	diff	baseline
ml	8.0MB	+116.3KB	7.9MB

page load bundle size

id	value	diff	baseline
ml	566.9KB	+66.5KB	500.4KB

History

💚 Build #67510 succeeded 73a6193
💚 Build #67469 succeeded 69bd3bf
💚 Build #67460 succeeded 90bde32
💔 Build #67454 failed 9d89bfa

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

darnautov · 2020-08-12T08:03:19Z

.../plugins/ml/public/application/components/annotations/annotations_table/annotations_table.js

    // if the annotation is at the series level
    // then pass the partitioning field(s) and detector index to the Single Metric Viewer
-    if (_.has(annotation, 'detector_index')) {
+    if (annotation.detector_index !== undefined) {


Not sure if it's important here, but has is not the same as check for undefined. Would be safer to replace with hasOwnProperty

I agree that has is not the same as checking for undefined, however the use of has in our code was not as a replacement for hasOwnProperty, it was only ever used to check whether a property had been set, and was not considering whether the property existed on the prototype chain.
In all other similar checks in our code base we use an undefined check, so for consistency i'd like to keep these changes.
but this is a good thing to point out!

darnautov

LGTM

* [ML] Removing full lodash library imports * more has * fixing missing filter * removing _ * removing unused file * removing first use * removing comment

…nes/processor-forms-a-d * 'master' of github.com:elastic/kibana: (25 commits) [ML] Removing full lodash library imports (elastic#74742) [Search] Server strategy example (elastic#71679) [Reporting] Fix and test for Listing of Reports (elastic#74453) [maps] fix drawing shapes (elastic#74689) [Resolver] Improve simulator. Add more click-through tests and panel tests. (elastic#74601) Deprecate schema-less specs in Vega (elastic#73805) [Security Solution] Rename Administration > Hosts subtab to Endpoints (elastic#74287) Timelion deprecation doc (elastic#74508) [Functional Tests] Adds a wait time between setting the index pattern and the time field on TSVB (elastic#74736) [Lens] Add styling options for x and y axes on the settings popover (elastic#71829) [Maps] add initial location option that fits to data bounds (elastic#74583) theme function (elastic#73451) [data.ui.query] Write filters to query log from default editor. (elastic#74474) [data.search.SearchSource] Move some SearchSource dependencies to the server. (elastic#74607) [Canvas][tech-debt] Convert renderers (elastic#74134) [security solutions][lists] Adds end to end tests (elastic#74473) pluralized for occurrences vs occurrence (elastic#74564) Update links that pointed to CONTRIBUTING.md (elastic#74757) [Ingest pipelines] Implement tabs in processor flyout (elastic#74469) [Event log] Use Alerts client & Actions client when fetching these types of SOs (elastic#73257) ... # Conflicts: # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/field_components/text_editor.tsx # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/manage_processor_form.tsx # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/append.tsx # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/bytes.tsx # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/circle.tsx # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/common_fields/field_name_field.tsx # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/common_fields/ignore_missing_field.tsx # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/convert.tsx # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/csv.tsx # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/date.tsx # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/date_index_name.tsx # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/dissect.tsx # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/dot_expander.tsx # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/drop.tsx # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/index.ts # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/shared.ts

* master: [Vega] add functional tests for Vega visualization (elastic#74097) [ML] Removing full lodash library imports (elastic#74742) [Search] Server strategy example (elastic#71679) [Reporting] Fix and test for Listing of Reports (elastic#74453) [maps] fix drawing shapes (elastic#74689) [Resolver] Improve simulator. Add more click-through tests and panel tests. (elastic#74601) Deprecate schema-less specs in Vega (elastic#73805) [Security Solution] Rename Administration > Hosts subtab to Endpoints (elastic#74287) Timelion deprecation doc (elastic#74508)

* [ML] Removing full lodash library imports * more has * fixing missing filter * removing _ * removing unused file * removing first use * removing comment

jgowdyelastic added 5 commits August 11, 2020 10:57

[ML] Removing full lodash library imports

11a6516

more has

9d89bfa

fixing missing filter

90bde32

removing _

69bd3bf

removing unused file

dc6d8ec

jgowdyelastic requested review from alvarezmelissa87, darnautov, qn895 and walterra August 11, 2020 11:10

jgowdyelastic self-assigned this Aug 11, 2020

jgowdyelastic added :ml chore refactoring v7.10.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes review labels Aug 11, 2020

jgowdyelastic marked this pull request as ready for review August 11, 2020 11:11

jgowdyelastic requested a review from a team as a code owner August 11, 2020 11:11

jgowdyelastic mentioned this pull request Aug 11, 2020

changing lodash #74539

Closed

removing first use

73a6193

alvarezmelissa87 reviewed Aug 11, 2020

View reviewed changes

alvarezmelissa87 approved these changes Aug 11, 2020

View reviewed changes

removing comment

e5e8d5d

darnautov reviewed Aug 12, 2020

View reviewed changes

darnautov approved these changes Aug 12, 2020

View reviewed changes

jgowdyelastic merged commit a81059b into elastic:master Aug 12, 2020

jgowdyelastic deleted the removing-full-lodash-library-imports branch August 12, 2020 08:24

jgowdyelastic mentioned this pull request Aug 12, 2020

[7.x] [ML] Removing full lodash library imports (#74742) #74832

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Removing full lodash library imports#74742

[ML] Removing full lodash library imports#74742
jgowdyelastic merged 7 commits intoelastic:masterfrom
jgowdyelastic:removing-full-lodash-library-imports

jgowdyelastic commented Aug 11, 2020 •

edited

Loading

Uh oh!

elasticmachine commented Aug 11, 2020

Uh oh!

alvarezmelissa87 Aug 11, 2020

Uh oh!

jgowdyelastic Aug 11, 2020

Uh oh!

alvarezmelissa87 left a comment

Uh oh!

kibanamachine commented Aug 11, 2020

Uh oh!

darnautov Aug 12, 2020

Uh oh!

jgowdyelastic Aug 12, 2020

Uh oh!

darnautov left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

jgowdyelastic commented Aug 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Aug 11, 2020

Uh oh!

alvarezmelissa87 Aug 11, 2020

Choose a reason for hiding this comment

Uh oh!

jgowdyelastic Aug 11, 2020

Choose a reason for hiding this comment

Uh oh!

alvarezmelissa87 left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Aug 11, 2020

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

async chunks size

page load bundle size

History

Uh oh!

darnautov Aug 12, 2020

Choose a reason for hiding this comment

Uh oh!

jgowdyelastic Aug 12, 2020

Choose a reason for hiding this comment

Uh oh!

darnautov left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jgowdyelastic commented Aug 11, 2020 •

edited

Loading