Value labels isValueContainedInElement breaks when value input is sent as number instead of string

[dej611]

Describe the bug
The isValueContainedInElement flag may crash the chart rendering in some scenarios.

To Reproduce
Steps to reproduce the behavior:

1. Go to the Bar Chart with value label story
2. Change the tickFormat prop in the LeftAxis component to return a number rather than a string (just remove the .toFixed(2) from the function code)
3. Click on 'contain value label within bar element'
4. 💥

Expected behaviour
The chart is rendered and the value labels are shown on the chart.

Screenshots

Version (please complete the following information):

• OS: Mac
• Browser: Chrome
• Elastic Charts: 21.0.0

Additional context
The API says the return value of the tickFormat function should be a string, but somehow TS managed to miss that and it happened to have a number output.

Update: The same error happens when no yAxis is associated with the current series, therefore no tickFormat function is used.

Not super important, but it would be helpful to be resilient to this kind of scenario, when possible.

Errors in browser console

text.ts:78 Uncaught TypeError: text.split is not a function
    at Object.wrapLines (text.ts:78)
    at Object.renderBarValues (bar.ts:72)
    at renderers.ts:161
    at Object.withContext (index.ts:36)
    at renderers.ts:158
    at index.ts:52
    at Array.forEach (<anonymous>)
    at Object.renderLayers (index.ts:52)
    at renderers.ts:69
    at Object.withContext (index.ts:36)
wrapLines @ text.ts:78
renderBarValues @ bar.ts:72
(anonymous) @ renderers.ts:161
withContext @ index.ts:36
(anonymous) @ renderers.ts:158
(anonymous) @ index.ts:52
renderLayers @ index.ts:52
(anonymous) @ renderers.ts:69
withContext @ index.ts:36
renderXYChartCanvas2d @ renderers.ts:41
push../src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx.XYChartComponent.drawCanvas @ xy_chart.tsx:133
push../src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx.XYChartComponent.componentDidUpdate @ xy_chart.tsx:119
commitLifeCycles @ react-dom.development.js:19835
commitLayoutEffects @ react-dom.development.js:22803
callCallback @ react-dom.development.js:188
invokeGuardedCallbackDev @ react-dom.development.js:237
invokeGuardedCallback @ react-dom.development.js:292
commitRootImpl @ react-dom.development.js:22541
unstable_runWithPriority @ scheduler.development.js:653
runWithPriority$1 @ react-dom.development.js:11039
commitRoot @ react-dom.development.js:22381
finishSyncRender @ react-dom.development.js:21807
performSyncWorkOnRoot @ react-dom.development.js:21793
(anonymous) @ react-dom.development.js:11089
unstable_runWithPriority @ scheduler.development.js:653
runWithPriority$1 @ react-dom.development.js:11039
flushSyncCallbackQueueImpl @ react-dom.development.js:11084
flushSyncCallbackQueue @ react-dom.development.js:11072
flushPassiveEffectsImpl @ react-dom.development.js:22883
unstable_runWithPriority @ scheduler.development.js:653
runWithPriority$1 @ react-dom.development.js:11039
flushPassiveEffects @ react-dom.development.js:22820
(anonymous) @ react-dom.development.js:22699
workLoop @ scheduler.development.js:597
flushWork @ scheduler.development.js:552
performWorkUntilDeadline @ scheduler.development.js:164
Show 6 more frames
scheduler.development.js:653 The above error occurred in the <XYChart> component:
    in XYChart (created by ConnectFunction)
    in ConnectFunction (created by ChartContainer)
    in div (created by ChartContainer)
    in ChartContainer (created by ConnectFunction)
    in ConnectFunction (created by Chart)
    in div (created by Chart)
    in div (created by Chart)
    in Provider (created by Chart)
    in Chart (created by storyFn)
    in div (created by Story)
    in Story (created by storyFn)
    in storyFn
    in ErrorBoundary

React will try to recreate this component tree from scratch using the error boundary you provided, ErrorBoundary.
logCapturedError @ react-dom.development.js:19527
logError @ react-dom.development.js:19564
update.payload @ react-dom.development.js:20723
getStateFromUpdate @ react-dom.development.js:12293
processUpdateQueue @ react-dom.development.js:12424
updateClassInstance @ react-dom.development.js:13185
updateClassComponent @ react-dom.development.js:17107
beginWork @ react-dom.development.js:18620
beginWork$1 @ react-dom.development.js:23179
performUnitOfWork @ react-dom.development.js:22154
workLoopSync @ react-dom.development.js:22130
performSyncWorkOnRoot @ react-dom.development.js:21756
(anonymous) @ react-dom.development.js:11089
unstable_runWithPriority @ scheduler.development.js:653
runWithPriority$1 @ react-dom.development.js:11039
flushSyncCallbackQueueImpl @ react-dom.development.js:11084
flushSyncCallbackQueue @ react-dom.development.js:11072
flushPassiveEffectsImpl @ react-dom.development.js:22883
unstable_runWithPriority @ scheduler.development.js:653
runWithPriority$1 @ react-dom.development.js:11039
flushPassiveEffects @ react-dom.development.js:22820
(anonymous) @ react-dom.development.js:22699
workLoop @ scheduler.development.js:597
flushWork @ scheduler.development.js:552
performWorkUntilDeadline @ scheduler.development.js:164
index.js:47 TypeError: text.split is not a function
    at Object.wrapLines (text.ts:78)
    at Object.renderBarValues (bar.ts:72)
    at renderers.ts:161
    at Object.withContext (index.ts:36)
    at renderers.ts:158
    at index.ts:52
    at Array.forEach (<anonymous>)
    at Object.renderLayers (index.ts:52)
    at renderers.ts:69
    at Object.withContext (index.ts:36)

Kibana Cross Issues
Add any Kibana related issues here: elastic/kibana#75426

[nickofthyme]

Thanks @dej611 for reporting this issue. Do you have an example tickFormat that allows this? I'm assuming it's just returning any type and thus conforming, but not really, to the type definition. But I'd be concerned if it were something like () => number and ts not flagging it.

[dej611]

In my case this was happening due to a wrong mapping of yAxis when creating the chart, so no TS expected error.
In the first investigation I thought it was a type problem and TS was just slow because of the project size. Later on I found it was not related to type, rather wrong id mapping.

[nickofthyme]

Ok good to know. I think in general it would be nice to gracefully handle js related errors but for now I think this is a lower priority.

This issue (#117) is related to improving error handling across the library but could be expanded on how to best handle errors.

[nickofthyme]

I'm going to close this issue as this is expected behavior, please feel free to reopen if you disagree.