fix: interval selection should gracefully fail if no valid projections #7442
Conversation
src/compile/selection/project.ts
Outdated
| @@ -63,15 +63,15 @@ const project: SelectionCompiler = { | |||
| }; | |||
|
|
|||
| const type = selCmpt.type; | |||
| const cfg = model.config.selection[type]; | |||
| const cfg = model.config.selection[type] as PointSelectionConfig; | |||
There was a problem hiding this comment.
What is selCmpt.type here? If it's telling us the type, do we still need as PointSelectionConfig?
There was a problem hiding this comment.
selCmpt.type is either point or interval. But, now, fields is only a property on the former, and encodings is on both. So, doing the typecast here felt like the cleanest fix to me rather than adding duplicative code throughout the rest of this file for a selCmpt.type === 'point' type guard. But I'm very open to other/better suggestions :)
There was a problem hiding this comment.
Alternatively, I can put this typecast in at L102:
-fields = cfg.fields;
+fields = (cfg as PointSelectionConfig).fields;Let me know if that's preferable.
There was a problem hiding this comment.
I think it's best not to assert a type when we don't know it yet. I changed it in 3987a99.
Partially addresses #7374. Our documentation already states that interval selections can only be projected via
encodingsbut our types/schema did not reflect that. On top of that, this PR allows the interval selection to fail more gracefully when invalid projections are provided — we always threw a warning about invalid projections, but now the spec will render out a (static) visualization instead of throwing obscure parse errors.