refactoring: introduce componentSpecFromYaml

[maxy-shpfy]

Description

Added a new componentSpecFromYaml function in utils/yaml.ts that safely parses YAML text into a ComponentSpec object with proper validation. This function replaces the direct use of yaml.load() in the component service, adding type checking and validation to prevent invalid component specifications.

Type of Change

• Improvement
• Cleanup/Refactor

Checklist

• I have tested this does not break current pipelines / runs functionality
• I have tested the changes on staging

Test Instructions

1. No functional changes to the app
2. No regression should be found across the app

[maxy-shpfy]

• refactoring: consolidate componentspec yaml parse load utils #1466
• refactoring: introduce componentSpecFromYaml #1460 👈 (View in Graphite)
• fix component text corruption in duplicate dialog #1448
• master

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

[Mbeaulne]

Do we need this? what are we using teh extra data for?

Can we just not return an error?

export function componentSpecFromYaml(yamlText: string): ComponentSpec {
  const loadedSpec = yaml.load(yamlText);
  if (typeof loadedSpec !== "object" || loadedSpec === null) {
    throw new Error("Invalid component specification format");
  }

  // todo: consider advanced validation
  if (!isValidComponentSpec(loadedSpec)) {
    throw new Error("Invalid component specification format");
  }

  return loadedSpec;
}

[maxy-shpfy]

I want to move away from using just "Errors". I`m ok to remove extra information if that would cause lot of debates (even though it's cheap and may help debugging just in case). But having named errors helps a lot while try/catch and overall error handling.

E.g. for this specific use-case failure of the parsing can be handled in some specific way https://github.com/TangleML/tangle-ui/blob/master/src/services/pipelineService.ts#L254

So . that's a more long-lasting move to have more precise error handling. Also when we start sending errors to Shopify Observe/Logs - having named errors becomes essential.

In nearest future we also should add linting rule to restrict using unnamed errors.

[camielvs]

hmm. I agree in principle with the idea, but I am not sure this PR specifically is the right place to start. Is it something we want to do gradually, or all at once?

Perhaps to unblock we would look at adding & using named errors as a separate PR and move to conversation over there?

[camielvs]

I like this PR. I can see the tech debt melting away before my eyes.

However, I do agree with the sentiment that this may not be the place to start introducing a Named Error system. I would like to see either:

1. Split named errors into its own PR
2. We follow this very rapidly to add named errors elsewhere and minimize the duration of any discrepancy and potential (new) tech debt

[maxy-shpfy]

Merge activity

• Dec 4, 5:28 PM UTC: A user started a stack merge that includes this pull request via Graphite.
• Dec 4, 5:29 PM UTC: Graphite rebased this pull request as part of a merge.
• Dec 4, 5:30 PM UTC: @maxy-shpfy merged this pull request with Graphite.