Skip to content

fix: v2 - component editor preview#2439

Open
maxy-shpfy wants to merge 1 commit into
masterfrom
06-18-fix_v2_-_component_editor_preview
Open

fix: v2 - component editor preview#2439
maxy-shpfy wants to merge 1 commit into
masterfrom
06-18-fix_v2_-_component_editor_preview

Conversation

@maxy-shpfy

@maxy-shpfy maxy-shpfy commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator

Description

Closes https://github.com/Shopify/oasis-frontend/issues/660

Replaces the usePreviewTaskNodeData hook and generateTaskSpec utility with a new pure function buildPreviewTaskNodeViewProps that directly maps component YAML into the v2 TaskNodeCard's prop interface. This removes the dependency on useQueryClient and the MobX spec model for rendering the component preview, allowing the preview to work without the editor's ReactFlow node context or query cache side effects.

The PreviewTaskNodeCard now wraps the v2 TaskNodeCard with ReactFlowProvider and SpecProvider instead of TaskNodeProvider, and drops the QueryErrorResetBoundary and Suspense wrappers since the new approach is fully synchronous.

Related Issue and Pull requests

Type of Change

  • Bug fix
  • New feature
  • Improvement
  • Cleanup/Refactor
  • Breaking change
  • Documentation update

Screenshots (if applicable)

Before After
image.png
image.png

Test Instructions

  1. Open the Component Editor and enter valid component YAML — confirm the preview card renders correctly.
  2. Enter invalid or incomplete YAML — confirm the preview card retains the last valid render rather than crashing.
  3. Enter YAML with no prior valid state — confirm a skeleton placeholder is shown.

Additional Comments

generateTaskSpec has been removed entirely as it is no longer needed outside of this preview context.

@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown

🎩 Preview

A preview build has been created at: 06-18-fix_v2_-_component_editor_preview/49e5720

Copy link
Copy Markdown
Collaborator Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@maxy-shpfy maxy-shpfy marked this pull request as ready for review June 19, 2026 01:25
@maxy-shpfy maxy-shpfy requested a review from a team as a code owner June 19, 2026 01:25
Comment on lines +23 to +57
return {
id: "preview",
entityId: "preview",
taskName: spec.name ?? "component-preview",
selected: false,
isHovered: false,
isSubgraph: isGraphImplementation(spec.implementation),
collapsed: false,
description: spec.description ?? "",
inputs: (spec.inputs ?? []).map((input) => ({
name: input.name,
type: input.type,
optional: input.optional,
default: input.default,
})),
outputs: (spec.outputs ?? []).map((output) => ({
name: output.name,
type: output.type,
})),
connectedInputNames: new Set<string>(),
connectedOutputNames: new Set<string>(),
annotations: [],
taskColor: undefined,
cacheDisabled: false,
digest: undefined,
inputDisplayValues: {},
isAggregator: false,
outputType: AggregatorOutputType.JsonArray,
onOutputTypeChange: noop,
onNodeClick: noop,
onInputClick: noop,
onOutputClick: noop,
onHandleClick: noop,
};
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm. manually listing everything out feels like a bit of a smell

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is typed, if contract changed - typecheck/lint will notify us. Not sure if there is a better justified way of building this object. In downstream PR we may try to extract "layout" portion of the TaskNodeCard to avoid usage of "handlers" , but I think it is too much for this specific use-case

Comment thread src/components/shared/ComponentEditor/components/PreviewTaskNodeCard.tsx Outdated
@maxy-shpfy maxy-shpfy force-pushed the 06-18-fix_v2_-_component_editor_preview branch from 272f8ce to 49e5720 Compare June 19, 2026 04:33
@maxy-shpfy maxy-shpfy requested a review from camielvs June 19, 2026 04:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants