Conversation
src/component/plotly-graph/parse.js
Outdated
|
|
||
| if (cont.type === 'table' && isPlainObj(cont.cells)) { | ||
| l = Math.max(l, findMaxArrayLength(cont.cells)) | ||
| } |
There was a problem hiding this comment.
table doesn't have any arrays in the top level container, does it? I might have put this in estimateDataLength then, alongside the similar logic for dimensions. Also this way we'd keep looking to recurse, trace.cells.cells.cells...
That said, the fact that this combines with the top level instead of replacing it is nice. Just imagining malicious users, the simple test if (Array.isArray(trace.dimensions)) as it is with an immediate return could be used on a trace that doesn't use dimensions to mask the fact that the real data arrays are huge.
So we could add a type qualifier to dimensions, but perhaps the most robust solution (and future-proof as we add more trace types so we don't need to worry too much about updating the logic here) would be to look at the top level AND in cells AND in dimensions regardless of trace type, and find the max length of everything. I don't think anyone is going to be bothered by the fact that their useless length-1e8 cells.junk array in a scatter trace got rejected 😈
There was a problem hiding this comment.
would be to look at the top level AND in cells AND in dimensions regardless of trace type, and find the max length of everything.
I like this solution a lot. Done in 2b5cf94
- look up the trace's "top" level and "dimensions" and "cells" regardless of trace type
|
Nice job! 💃 |
2 participants
I missed out on
tabletraces in #55, this PR adds them in.cc @nicolaskruchten