isFulfilledData should only consider `value` and not `open`

[mozeryansky]

Lightweight Charts™ Version: latest

Following up from my question in discussions I encounter this as a bug often.

As Whitespace is a data item without a value from here: https://tradingview.github.io/lightweight-charts/docs/api/interfaces/WhitespaceData. However the function which I believe is where this is checked considers if open is also present and will throw and error: https://github.com/tradingview/lightweight-charts/blob/master/src/model/data-consumer.ts#L188

[SlicedSilver]

It depends on the type of series that the data is intended for. The library expects one of the following:

• 'Line type' data, which has value
• or 'Bar type' data, which has open, high, low, close

As you can see there is no overlap of keys thus it is safe to check if the data is fulfilled by checking if either open or value are supplied. You shouldn't be supplying partially complete bars, or extra information in the data. If you do need to use extra info then make use of the customValues key.

[mozeryansky]

I see that and I may not be using the API as intended, but it does seem like an unintended side effect. As I start with my candles and I compute my signal and add that as a value. Then I pass the new data array as the argument for my line.

Psuedocode:

candles = getCandles('TSLA')
sma = addSMAValue(candles)

candleStickSeries.setData(candles)
lineSeries.setData(sma)

However, for the first values in the line series with undefined values (like sma_30) will error out saying "... series item data value of open must be a number ... " from here, since the validator will be true as (open OR value) is present.

Specifically, I had to change my code as follows:

// FROM: errors because `open` is present
const data = candles.map((bar, i) => {
    const value = smaValues[i - diff]
    return value ? { ...bar, value } : bar
})

// TO: when value is invalid, only include time
const data = candles.map((v, i) => {
    const value = smaValues[i - diff]
    return value ? { ...v, value } : { time: v.time }
})

I ask, because it seems it's a bug for line series to tell me open is causing an issue even though it never uses that value for anything and causes an exception to be thrown.

---

Edit: The assert is from the checkLineItem function here, which uses the same validation as checkBarItem.

[mozeryansky]

I guess I'm just asking if there could instead be a isFulfilledBarData and isFulfilledLineData instead of only a isFulfilledData? I can make a PR if you think this is something to be considered.

[SlicedSilver]

In principle it should be fine to add a different checker for each data type. It would probably only add a tiny bit to the bundle size and be easy to implement. So if you create a PR and the increase to the bundle size is tiny then we would consider merging it.

Whether it is required is still debatable because the types for LineData wouldn't actually let you have open or other extra properties. If you tried this in Typescript then you would get a compiler error. I know not everyone uses TS, so this issue wouldn't be known until runtime for vanilla JS developers.

The suggested approach would be to modify your addSMAValue function such that it returns only data containing time and value (optional) instead of adding value to existing bar data. You wouldn't want to be mutating the existing candles data anyway so if you are creating a new array for the sma values then it should also be easy to only include the expected properties.

Additionally, we actually already have a code sample for an SMA indicator on candles data here: https://jsfiddle.net/TradingView/537kjtfg/ It was written a few years ago, and you can ignore most of the code (which is displaying a legend, and generating random but realistic candle data), but you can pay attention to the calculateSMA function

[mozeryansky]

I guess I've already written the code to avoid the issue so it's solved for me in a way, but when doing something quick I keep running into it. I use TS but I didn't see a compiler error, I'll look into that too. I should have time soon to try out a PR and see how the build size changes, it'll give me a better understanding of that portion of the code too. Thanks.

[mozeryansky]

I've created a PR. Let me know what you think, thanks!