[ui] Upgrade yaml library, improve config merge behavior (#20055)

## Summary & Motivation

This PR aims to address #19668 (but doesn't entirely fix it):

Context: In the launchpad, there are several scenarios where we parse
the current YAML, modify it as a JS object tree, and serialize it back
to YAML. This is fundamentally undesirable because some YAML values (eg:
5.0) experience type coercion (eg: to 5) because JS does not have a
Float type, etc. Fully fixing this would require changing the YAML merge
logic to use YAML ASTs, but that's complex and we express config
defaults as JS objects, so it wouldn't entirely resolve the "5.0 to 5"
issue.

This PR fixes two of the more straightforward bugs in the "parse, merge,
stringify" workflow:

- The deepmerge library changed in v3+ to concatenate arrays by default
rather than combining them (so [a,b,c] + [c,d,e] = [a,b,c,c,d,e]). This
changes it to the expected ([a,b,c,d,e]), and means that merging two
identical arrays does nothing to them.

- The yaml stringify method tries to be "lean" with quotes, but can
un-quote some values that other YAML parsers parse as other types. The
example called out in #19668 was "5:30" becoming 5:30, which is a
Sexagesimal base-60 number in python. (This has been called out as awful
behavior before: https://news.ycombinator.com/item?id=26679229). I
changed the serialize call to explicitly maintain quotes on all string
values, which looks a bit ugly if you're a YAML purist but will avoid
these cases.

## How I Tested These Changes

I broke out a yamlMerge helper and wrote tests for the cases above,
verified they failed and then fixed them

---------

Co-authored-by: bengotow <[email protected]>

js_modules/dagster-ui/packages/ui-components/package.json

@@ -37,7 +37,7 @@
     "codemirror": "^5.65.2",
     "deepmerge": "^4.2.2",
     "react-virtualized": "^9.22.3",
-    "yaml": "2.2.2"
+    "yaml": "2.4.0"
   },
   "devDependencies": {
     "@babel/core": "^7.21.8",

js_modules/dagster-ui/packages/ui-core/package.json

@@ -69,7 +69,7 @@
     "strip-markdown": "^6.0.0",
     "subscriptions-transport-ws": "^0.9.15",
     "worker-loader": "^3.0.8",
-    "yaml": "2.2.2"
+    "yaml": "2.4.0"
   },
   "devDependencies": {
     "@apollo/client": "3.7.13",

js_modules/dagster-ui/packages/ui-core/src/launchpad/LaunchpadSession.tsx

@@ -19,7 +19,6 @@ import {
   TextInput,
   isHelpContextEqual,
 } from '@dagster-io/ui-components';
-import merge from 'deepmerge';
 import uniqBy from 'lodash/uniqBy';
 import * as React from 'react';
 import styled from 'styled-components';
@@ -53,6 +52,7 @@ import {
   PreviewConfigQuery,
   PreviewConfigQueryVariables,
 } from './types/LaunchpadSession.types';
+import {mergeYaml, sanitizeConfigYamlString} from './yamlUtils';
 import {showCustomAlert} from '../app/CustomAlertProvider';
 import {
   IExecutionSession,
@@ -282,14 +282,9 @@ const LaunchpadSession = (props: LaunchpadSessionProps) => {
       return false;
     }
     try {
-      const defaultsYaml = yaml.parse(sanitizeConfigYamlString(rootDefaultYaml));
-
-      const currentUserConfig = yaml.parse(sanitizeConfigYamlString(currentSession.runConfigYaml));
-      const updatedRunConfigData = merge(defaultsYaml, currentUserConfig);
-
       return (
-        yaml.stringify(currentUserConfig, {sortMapEntries: true}) !==
-        yaml.stringify(updatedRunConfigData, {sortMapEntries: true})
+        mergeYaml(rootDefaultYaml, currentSession.runConfigYaml, {sortMapEntries: true}) !==
+        mergeYaml({}, currentSession.runConfigYaml, {sortMapEntries: true})
       );
     } catch (err) {
       return false;
@@ -299,24 +294,15 @@ const LaunchpadSession = (props: LaunchpadSessionProps) => {
   const onScaffoldMissingConfig = () => {
     const config = runConfigSchema ? scaffoldPipelineConfig(runConfigSchema) : {};
     try {
-      const runConfigData = yaml.parse(sanitizeConfigYamlString(currentSession.runConfigYaml));
-      const updatedRunConfigData = merge(config, runConfigData);
-      const runConfigYaml = yaml.stringify(updatedRunConfigData);
-      onSaveSession({runConfigYaml});
+      onSaveSession({runConfigYaml: mergeYaml(config, currentSession.runConfigYaml)});
     } catch (err) {
       showCustomAlert({title: 'Invalid YAML', body: YAML_SYNTAX_INVALID});
     }
   };
 
   const onExpandDefaults = () => {
     if (rootDefaultYaml) {
-      const defaultsYaml = yaml.parse(sanitizeConfigYamlString(rootDefaultYaml));
-
-      const currentUserConfig = yaml.parse(sanitizeConfigYamlString(currentSession.runConfigYaml));
-      const updatedRunConfigData = merge(defaultsYaml, currentUserConfig);
-      const mergedYaml = yaml.stringify(updatedRunConfigData);
-
-      onSaveSession({runConfigYaml: mergedYaml});
+      onSaveSession({runConfigYaml: mergeYaml(rootDefaultYaml, currentSession.runConfigYaml)});
     }
   };
 
@@ -520,12 +506,7 @@ const LaunchpadSession = (props: LaunchpadSessionProps) => {
           body: <PythonErrorInfo error={partition.runConfigOrError} />,
         });
       } else {
-        runConfigYaml = yaml.stringify(
-          merge(
-            yaml.parse(sanitizeConfigYamlString(currentSession.runConfigYaml)),
-            yaml.parse(sanitizeConfigYamlString(partition.runConfigOrError.yaml)),
-          ),
-        );
+        runConfigYaml = mergeYaml(currentSession.runConfigYaml, partition.runConfigOrError.yaml);
       }
 
       const solidSelection = sessionSolidSelection || partition.solidSelection;
@@ -894,8 +875,6 @@ const deletePropertyPath = (obj: any, path: string) => {
   }
 };
 
-const sanitizeConfigYamlString = (yamlString: string) => (yamlString || '').trim() || '{}';
-
 const PREVIEW_CONFIG_QUERY = gql`
   query PreviewConfigQuery(
     $pipeline: PipelineSelector!

js_modules/dagster-ui/packages/ui-core/src/launchpad/__tests__/yamlUtils.test.ts

@@ -0,0 +1,30 @@
+import {mergeYaml} from '../yamlUtils';
+
+describe('yamlUtils.mergeYaml', () => {
+  it('should not duplicate array contents', () => {
+    const a = `hello:
+  - 1
+  - a: "object"
+  - 3`;
+
+    const b = `hello:
+  - 1
+  - a: "object"
+  - 4`;
+
+    const c = `hello:
+  - 1
+  - a: 'object'
+  - 3
+  - 4
+`;
+    expect(mergeYaml(a, b)).toEqual(c);
+  });
+
+  it('should quote strings that may be mis-interpreted', () => {
+    const a = `hello: "5:30"`;
+    const b = `world: 4`;
+    const c = `hello: '5:30'\nworld: 4\n`;
+    expect(mergeYaml(a, b)).toEqual(c);
+  });
+});

js_modules/dagster-ui/packages/ui-core/src/launchpad/yamlUtils.ts

@@ -0,0 +1,46 @@
+import merge from 'deepmerge';
+import * as yaml from 'yaml';
+
+export const sanitizeConfigYamlString = (yamlString: string) => (yamlString || '').trim() || '{}';
+
+/**
+ * Utility function to merge two YAML documents:
+ * - Stringify string values with quotes. Avoids known issues with "5:30" becoming 5:30 which parses as a Sexagesimal number
+ * - When merging arrays, combine rather than concatenate (the default deepmerge behavior)
+ * -
+ */
+export function mergeYaml(
+  base: string | Record<string, any>,
+  overrides: string | Record<string, any>,
+  extraOpts?: yaml.SchemaOptions,
+) {
+  const baseObj = typeof base === 'string' ? yaml.parse(sanitizeConfigYamlString(base)) : base;
+
+  const overridesObj =
+    typeof overrides === 'string' ? yaml.parse(sanitizeConfigYamlString(overrides)) : overrides;
+
+  const arrayCombineMerge: merge.Options['arrayMerge'] = (target, source, options) => {
+    const destination = target.slice();
+
+    source.forEach((item, index) => {
+      if (typeof destination[index] === 'undefined') {
+        destination[index] = options?.cloneUnlessOtherwiseSpecified(item, options);
+      } else if (options?.isMergeableObject(item)) {
+        destination[index] = merge(target[index], item, options);
+      } else if (target.indexOf(item) === -1) {
+        destination.push(item);
+      }
+    });
+    return destination;
+  };
+
+  const mergedObj = merge(baseObj, overridesObj, {
+    arrayMerge: arrayCombineMerge,
+  });
+
+  return yaml.stringify(mergedObj, {
+    defaultKeyType: 'PLAIN',
+    defaultStringType: 'QUOTE_SINGLE',
+    ...extraOpts,
+  });
+}

js_modules/dagster-ui/yarn.lock

@@ -2489,7 +2489,7 @@ __metadata:
     storybook: "npm:^7.4.5"
     typescript: "npm:5.3.2"
     webpack: "npm:^5.88.1"
-    yaml: "npm:2.2.2"
+    yaml: "npm:2.4.0"
   peerDependencies:
     "@blueprintjs/core": ^4.20.2
     "@blueprintjs/popover2": 1.13.12
@@ -2627,7 +2627,7 @@ __metadata:
     typescript: "npm:5.3.2"
     webpack: "npm:^5.88.1"
     worker-loader: "npm:^3.0.8"
-    yaml: "npm:2.2.2"
+    yaml: "npm:2.4.0"
   peerDependencies:
     "@apollo/client": 3.7.13
     "@blueprintjs/core": ^4.20.2
@@ -23932,10 +23932,12 @@ __metadata:
   languageName: node
   linkType: hard
 
-"yaml@npm:2.2.2":
-  version: 2.2.2
-  resolution: "yaml@npm:2.2.2"
-  checksum: 991384bf875dd1df9ab8e3393d6db7010d222eef4e750f17590b5d3d96d3f3ad49420f58fff6a3896321fb64d08f4bb3b4edf0d337fdfd6e960119ef5aa0527c
+"yaml@npm:2.4.0":
+  version: 2.4.0
+  resolution: "yaml@npm:2.4.0"
+  bin:
+    yaml: bin.mjs
+  checksum: 60cdb4499365a9649b4fb5cfbc496a1cd415013166f1c97aef7bf002caa0ad146058c3af91dda94c5799085d8728f1e8859b3854897ecebd10b84d7b054f0a40
   languageName: node
   linkType: hard