fix: use unvalidated spec as fallback for canvas#9186
Conversation
runtime/canvases.go
Outdated
| // Use the valid spec if available, otherwise fall back to the unvalidated spec. | ||
| // Falling back allows Rill Developer to keep the canvas accessible while the user is mid-edit. | ||
| spec := res.GetCanvas().State.ValidSpec | ||
| if spec == nil { | ||
| spec = res.GetCanvas().Spec | ||
| } |
There was a problem hiding this comment.
This feels problematic because a) it means we may end up serving an invalid canvas in Rill Cloud, b) it may break features if we add rewriting logic in the canvas/component reconcilers in the future (like we have for metrics views and explores).
There was a problem hiding this comment.
If the visual editor needs this, can we guard it behind an unsafe bool parameter in the ResolveCanvasRequest payload? And then make sure unsafe is only sent by the visual editor, never in Rill Cloud and never by the readonly previews?
| super(resource, parent, path, baseSpec as TConfig); | ||
|
|
||
| this.type = resource.component?.state?.validSpec?.renderer as ChartType; | ||
| this.type = (resource.component?.state?.validSpec?.renderer ?? |
There was a problem hiding this comment.
Is this code shared with readonly/cloud components? It's important that code that serves prod/readonly dashboards only ever read validSpec, never the raw spec (because we don't offer any guarantees about the spec, and may need to add rewriting logic at some later point, which would break code that relies on the unvalidated raw spec).
begelundmuller
left a comment
begelundmuller
left a comment
There was a problem hiding this comment.
Looks good to me :)
* fix: use unvalidated spec as fallback for canvas * add unsafe bool for serving unvalidated spec
2 participants
Previously, if any canvas component had an invalid spec, the entire canvas dashboard became inaccessible in Rill Developer because
ResolveCanvasreturned early whencanvas.state.validSpecwas nil. This broke the editing workflow where a user intentionally changes ametrics_view(causing a transient error state) before selecting the correct fields.specinResolveCanvaswhenvalidSpecis nil, so the canvas shell remains accessible during mid-edit error statesBaseChart,CanvasPivot, andBaseCanvasComponentspec.rendererwhen reading component type incanvas-entity.tsandBaseChart/CanvasPivot, fixing a blank inspector panel when clicking a broken componentThe error/loading banner UX in
CanvasInitialization.svelteis unchanged — it still usesvalidSpecto drive reconcile error messaging correctly. This matches the fallback pattern already used inResolveTransitiveAccess(reconcilers/canvas.go:169-172).Checklist: