build: update dependencies #962

markov00 · 2021-01-05T14:10:48Z

Summary

Upgrading dependencies:

Jest related: all jest related packages. This required an updated version of TS version, for that commit increased to 3.9 (fixing few ts errors introduced by that upgraded version). jest-puppeteer and puppeteer were not upgraded due to lack of maintenance of the jest-puppeteer package: this is anyway only used for debugging VRTs that are not actually used frequently.
ESLint related packages: these also required a code cleanup in some parts
CLI tools like semantic-release and commitlint
Time-related libs like Luxon and moment
Typescript to 4.1.3

Notes

It easier to review the PR by single commit as they are mostly self-contained

Next steps

On separate PRs:

upgrade EUI
upgrade webpack (to latest 4), and related packages
upgrade @elastic/eui
storybook (addon-info needs to be replaced, the upgrade should be smooth just remember to override the page style padding for the story due to an additional 20px padding in SB6)
remove unused/deprecated deps like core-js, babel-polyfill, @elastic/ghithub-check
breaking-change: compile everything on ES2020 (the most updated target version compatible with most desktop browsers)

Checklist

Delete any items that are not applicable to this PR.

Any consumer-facing exports were added to src/index.ts (and stories only import from ../src except for test data & storybook)
This was checked for cross-browser compatibility
Proper documentation or storybook story was added for features that require explanation or tutorials
Unit tests were updated or added to match the most common scenarios

The commit updates all jest related files. Increase TS version to 3.9 to align with the jest required versions.

codecov-io · 2021-01-05T14:34:14Z

Codecov Report

Merging #962 (720e0bf) into master (ffd626b) will increase coverage by 0.25%.
The diff coverage is 48.75%.

@@            Coverage Diff             @@
##           master     #962      +/-   ##
==========================================
+ Coverage   70.60%   70.85%   +0.25%     
==========================================
  Files         344      360      +16     
  Lines       11056    10596     -460     
  Branches     2392     2167     -225     
==========================================
- Hits         7806     7508     -298     
+ Misses       3236     3000     -236     
- Partials       14       88      +74

Flag	Coverage Δ
unittests	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...pes/goal_chart/renderer/canvas/canvas_renderers.ts	`10.44% <0.00%> (+1.05%)`	⬆️
src/chart_types/goal_chart/state/chart_state.tsx	`57.89% <ø> (+0.99%)`	⬆️
...oal_chart/state/selectors/on_element_out_caller.ts	`31.81% <ø> (+1.38%)`	⬆️
.../chart_types/heatmap/layout/viewmodel/viewmodel.ts	`4.57% <0.00%> (+0.08%)`	⬆️
...es/heatmap/renderer/canvas/connected_component.tsx	`42.85% <ø> (+0.85%)`	⬆️
src/chart_types/heatmap/state/chart_state.tsx	`63.23% <ø> (ø)`
.../state/selectors/get_brushed_highlighted_shapes.ts	`33.33% <ø> (+2.56%)`	⬆️
...t_types/heatmap/state/selectors/get_color_scale.ts	`30.00% <0.00%> (+1.87%)`	⬆️
...heatmap/state/selectors/on_element_click_caller.ts	`32.14% <0.00%> (+2.14%)`	⬆️
...types/partition_chart/layout/types/config_types.ts	`100.00% <ø> (ø)`
... and 256 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ffd626b...720e0bf. Read the comment docs.

monfera

Looks great 🎉 especially fond of the code simplifications like unnested and/or removed ifs in favor of flat, and more expression-oriented ways.

Would be great to add the mentioned further linting relaxations but that could be a future PR too.

A related change, maybe in this PR but probably a separate one, is the switch to ES2015 as the target. A separate PR for that makes it easier to revert on the offchance it's not digested well by Kibana, should I put in one?

In the context of this PR, only the addition of empty rows between one-liner class members (properties, methods) looks disadvantageous, at least I often work on the notebook screen alone and prefer to see as much of the class vertically (and more code in general) as possible, but I can't really choose a smaller font size, so I like the density where the code itself is very lightweight, regular and uncompicated. Here's hoping it's not a linting rule that makes us do this 😄

src/chart_types/partition_chart/layout/utils/group_by_rollup.ts

.eslintrc.js

monfera · 2021-01-07T09:46:25Z

.eslintrc.js

+    'unicorn/no-array-callback-reference': 'off',
+    'unicorn/no-array-reduce': 'off',
+    'unicorn/prefer-dom-node-append': 'off',
+    'unicorn/prefer-dom-node-remove': 'off',


Good calls. Would be useful to switch off the stricter level (property setting) of no-param-reassign, it's routinely needed by efficient data processing and the Canvas2d API alike.

Also, allowing a comment line to be present atop of the file, eg. right after the comments block. And eg. mocks need no formatting, and sometimes a long array literal shouldn't be broken to a lot of lines either. Currently a simple bypass may require an exemptions inception for that bypass to work, some of these rules seem to buy less than what's lost in productivity, flow and developer experience.

Would be great to tame the destructuring rules too while at it, for at least arrays

integration/page_objects/common.ts

src/chart_types/goal_chart/state/selectors/on_element_click_caller.ts

monfera · 2021-01-07T09:57:26Z

src/chart_types/heatmap/layout/viewmodel/viewmodel.ts

+          ...textValue,
+          x: chartDimensions.left + (xScale(textBox) || 0) + xScale.bandwidth() / 2,
+        };
+      });


Nice! As we don't reuse textValue, it doesn't give away its data flow simplicity on 1st look; this'd work to reflect that, can go either way of course

: xValues.map<TextBox>((textBox: any) => ({ x: chartDimensions.left + (xScale(textBox) || 0) + xScale.bandwidth() / 2, ...getTextValue(config.xAxisLabel.formatter)(textBox) })

done in 720e0bf

monfera · 2021-01-07T09:59:45Z

src/chart_types/partition_chart/layout/types/geometry_types.ts

@@ -47,7 +47,9 @@ export type PointTuples = [PointTuple, ...PointTuple[]]; // at least one point
 /** @internal */
 export class Circline {
  x: Coordinate = NaN;
+


Is it aesthetic, or helps code reading? In general I find that simple one-liners are better kept compact. It can go either way even if I'd prefer at least the option of dense, compact blocks for the simple things

This was imposed by the eslint/prettier. I can disable this rule and lint all the code on a future PR

src/chart_types/partition_chart/layout/utils/color_library_wrappers.ts

src/chart_types/partition_chart/renderer/canvas/partition.tsx

monfera · 2021-01-07T10:08:41Z

src/chart_types/xy_chart/utils/axis_utils.test.ts

@@ -1542,8 +1540,8 @@ describe('Axis computational utils', () => {
    ]);
  });
  test('should show unique consecutive ticks if duplicateTicks is set to false', () => {
-    const formatter: TickFormatter = (d, options = { timeZone: 'utc+1' }) =>
-      DateTime.fromMillis(d, { setZone: true, zone: options.timeZone }).toFormat('HH:mm');
+    const formatter: TickFormatter = (d, { timeZone } = { timeZone: 'utc+1' }) =>


I suppose the goal is to cover the case when options isn't supplied at all, rather than the case when it is provided as an object which doesn't have a timeZone property, that's why it's not { timeZone = 'utc+1' }

I think I've messed up the code with the eslinter in this case.
I've fixed in 720e0bf

rshen91

Thank you for making these changes. They look good to me

nickofthyme

LGTM, just had two questions

integration/helpers.ts

src/components/legend/legend.tsx

markov00 · 2021-01-30T15:42:55Z

🎉 This PR is included in version 24.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

build(jest): update jest related packages

46f7336

The commit updates all jest related files. Increase TS version to 3.9 to align with the jest required versions.

markov00 changed the title ~~build(jest): update jest related packages~~ build: update dependencies Jan 5, 2021

refactor: fix lint errors after deps upgrade

68065a7

markov00 force-pushed the 2021_01_05-dep_update branch from 430de47 to 68065a7 Compare January 5, 2021 20:19

markov00 added 2 commits January 5, 2021 21:47

build: update eslint related packages

1ae62a1

build: upgrade CLI tools

fbb41bf

markov00 force-pushed the 2021_01_05-dep_update branch from 1b3481c to fbb41bf Compare January 5, 2021 22:28

markov00 added 2 commits January 5, 2021 23:31

build: upgrade time related packages

4e76488

build: upgrade TS to 4.1

82c05bf

markov00 force-pushed the 2021_01_05-dep_update branch from d1737c5 to 82c05bf Compare January 6, 2021 13:03

markov00 marked this pull request as ready for review January 6, 2021 15:16

markov00 requested review from nickofthyme and rshen91 January 6, 2021 15:16

monfera approved these changes Jan 7, 2021

View reviewed changes

rshen91 approved these changes Jan 7, 2021

View reviewed changes

markov00 added 2 commits January 8, 2021 17:34

Merge branch 'master' into 2021_01_05-dep_update

6fe404b

fix: cleanup code linting from review comments

720e0bf

nickofthyme approved these changes Jan 11, 2021

View reviewed changes

integration/helpers.ts Show resolved Hide resolved

src/components/legend/legend.tsx Show resolved Hide resolved

markov00 merged commit 3b6865c into elastic:master Jan 11, 2021

markov00 deleted the 2021_01_05-dep_update branch January 11, 2021 15:26

markov00 mentioned this pull request Jan 29, 2021

Compile to ES2020 #998

Open

11 tasks

markov00 added the released Issue released publicly label Jan 30, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: update dependencies #962

build: update dependencies #962

markov00 commented Jan 5, 2021 •

edited

Loading

codecov-io commented Jan 5, 2021 •

edited

Loading

monfera left a comment •

edited

Loading

monfera Jan 7, 2021

monfera Jan 7, 2021

markov00 Jan 8, 2021

monfera Jan 7, 2021

markov00 Jan 8, 2021

monfera Jan 7, 2021

markov00 Jan 8, 2021

rshen91 left a comment

nickofthyme left a comment •

edited

Loading

markov00 commented Jan 30, 2021

build: update dependencies #962

build: update dependencies #962

Conversation

markov00 commented Jan 5, 2021 • edited Loading

Summary

Notes

Next steps

Checklist

codecov-io commented Jan 5, 2021 • edited Loading

Codecov Report

monfera left a comment • edited Loading

Choose a reason for hiding this comment

monfera Jan 7, 2021

Choose a reason for hiding this comment

monfera Jan 7, 2021

Choose a reason for hiding this comment

markov00 Jan 8, 2021

Choose a reason for hiding this comment

monfera Jan 7, 2021

Choose a reason for hiding this comment

markov00 Jan 8, 2021

Choose a reason for hiding this comment

monfera Jan 7, 2021

Choose a reason for hiding this comment

markov00 Jan 8, 2021

Choose a reason for hiding this comment

rshen91 left a comment

Choose a reason for hiding this comment

nickofthyme left a comment • edited Loading

Choose a reason for hiding this comment

markov00 commented Jan 30, 2021

markov00 commented Jan 5, 2021 •

edited

Loading

codecov-io commented Jan 5, 2021 •

edited

Loading

monfera left a comment •

edited

Loading

nickofthyme left a comment •

edited

Loading