Only render JSON objects and arrays, not primitives (#1366)

kubeflow · May 22, 2019 · b1a6848 · b1a6848
1 parent d160425
commit b1a6848
Show file tree

Hide file tree

Showing 3 changed files with 209 additions and 2 deletions.
diff --git a/frontend/src/components/DetailsTable.test.tsx b/frontend/src/components/DetailsTable.test.tsx
@@ -60,6 +60,39 @@ describe('DetailsTable', () => {
     expect(tree).toMatchSnapshot();
   });
 
+  it('does render arrays as JSON', () => {
+    const tree = shallow(<DetailsTable fields={[['key', '[]']]} />);
+    expect(tree).toMatchSnapshot();
+  });
+
+  it('does render arrays as JSON', () => {
+    const tree = shallow(<DetailsTable fields={[['key', '{}']]} />);
+    expect(tree).toMatchSnapshot();
+  });
+
+  it('does not render nulls as JSON', () => {
+    const tree = shallow(<DetailsTable fields={[['key', 'null']]} />);
+    expect(tree).toMatchSnapshot();
+  });
+
+  it('does not render numbers as JSON', () => {
+    const tree = shallow(<DetailsTable fields={[['key', '10']]} />);
+    expect(tree).toMatchSnapshot();
+  });
+
+  it('does not render strings as JSON', () => {
+    const tree = shallow(<DetailsTable fields={[['key', '"some string"']]} />);
+    expect(tree).toMatchSnapshot();
+  });
+
+  it('does not render booleans as JSON', () => {
+    const tree = shallow(<DetailsTable fields={[
+      ['key1', 'true'],
+      ['key2', 'false']
+    ]} />);
+    expect(tree).toMatchSnapshot();
+  });
+
   it('shows keys and values for multiple rows', () => {
     const tree = shallow(<DetailsTable fields={[
       ['key1', 'value1'],

diff --git a/frontend/src/components/DetailsTable.tsx b/frontend/src/components/DetailsTable.tsx
@@ -54,8 +54,13 @@ export default (props: DetailsTableProps) => {
     {!!props.title && <div className={commonCss.header}>{props.title}</div>}
     <div>
       {props.fields.map((f, i) => {
-        try{
+        try {
           const parsedJson = JSON.parse(f[1]);
+          // Nulls, booleans, strings, and numbers can all be parsed as JSON, but we don't care
+          // about rendering those using CodeMirror. Note that `typeOf null` returns 'object'
+          if (parsedJson === null || typeof parsedJson !== 'object') {
+            throw new Error('Parsed JSON was neither an array nor an object. Using default renderer');
+          }
           return (
             <div key={i} className={css.row}>
               <span className={css.key}>{f[0]}</span>
@@ -74,7 +79,7 @@ export default (props: DetailsTableProps) => {
             </div>
           );
         } catch (err) {
-          // If the value isn't JSON, just display it as is
+          // If the value isn't a JSON object, just display it as is
           return (
             <div key={i} className={css.row}>
               <span className={css.key}>{f[0]}</span>

diff --git a/frontend/src/components/__snapshots__/DetailsTable.test.tsx.snap b/frontend/src/components/__snapshots__/DetailsTable.test.tsx.snap
@@ -1,5 +1,174 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`DetailsTable does not render booleans as JSON 1`] = `
+<Fragment>
+  <div>
+    <div
+      className="row"
+      key="0"
+    >
+      <span
+        className="key"
+      >
+        key1
+      </span>
+      <span
+        className="valueText"
+      >
+        true
+      </span>
+    </div>
+    <div
+      className="row"
+      key="1"
+    >
+      <span
+        className="key"
+      >
+        key2
+      </span>
+      <span
+        className="valueText"
+      >
+        false
+      </span>
+    </div>
+  </div>
+</Fragment>
+`;
+
+exports[`DetailsTable does not render nulls as JSON 1`] = `
+<Fragment>
+  <div>
+    <div
+      className="row"
+      key="0"
+    >
+      <span
+        className="key"
+      >
+        key
+      </span>
+      <span
+        className="valueText"
+      >
+        null
+      </span>
+    </div>
+  </div>
+</Fragment>
+`;
+
+exports[`DetailsTable does not render numbers as JSON 1`] = `
+<Fragment>
+  <div>
+    <div
+      className="row"
+      key="0"
+    >
+      <span
+        className="key"
+      >
+        key
+      </span>
+      <span
+        className="valueText"
+      >
+        10
+      </span>
+    </div>
+  </div>
+</Fragment>
+`;
+
+exports[`DetailsTable does not render strings as JSON 1`] = `
+<Fragment>
+  <div>
+    <div
+      className="row"
+      key="0"
+    >
+      <span
+        className="key"
+      >
+        key
+      </span>
+      <span
+        className="valueText"
+      >
+        "some string"
+      </span>
+    </div>
+  </div>
+</Fragment>
+`;
+
+exports[`DetailsTable does render arrays as JSON 1`] = `
+<Fragment>
+  <div>
+    <div
+      className="row"
+      key="0"
+    >
+      <span
+        className="key"
+      >
+        key
+      </span>
+      <UnControlled
+        className="valueJson"
+        editorDidMount={[Function]}
+        options={
+          Object {
+            "gutters": Array [
+              "codeMirrorGutter",
+            ],
+            "lineWrapping": true,
+            "mode": "application/json",
+            "readOnly": true,
+            "theme": "default",
+          }
+        }
+        value="[]"
+      />
+    </div>
+  </div>
+</Fragment>
+`;
+
+exports[`DetailsTable does render arrays as JSON 2`] = `
+<Fragment>
+  <div>
+    <div
+      className="row"
+      key="0"
+    >
+      <span
+        className="key"
+      >
+        key
+      </span>
+      <UnControlled
+        className="valueJson"
+        editorDidMount={[Function]}
+        options={
+          Object {
+            "gutters": Array [
+              "codeMirrorGutter",
+            ],
+            "lineWrapping": true,
+            "mode": "application/json",
+            "readOnly": true,
+            "theme": "default",
+          }
+        }
+        value="{}"
+      />
+    </div>
+  </div>
+</Fragment>
+`;
+
 exports[`DetailsTable shows a row with a title 1`] = `
 <Fragment>
   <div