Fix null contentRef handling in RecordColumnValue (#3566)

Co-authored-by: Nicolas Dorseuil <[email protected]>

Author: conico974

packages/gitbook/src/components/DocumentView/Table/RecordColumnValue.tsx

@@ -15,12 +15,17 @@ import { getSimplifiedContentType } from '@/lib/files';
 import { resolveContentRef } from '@/lib/references';
 import { tcls } from '@/lib/tailwind';
 import { filterOutNullable } from '@/lib/typescript';
-
 import type { BlockProps } from '../Block';
 import { Blocks } from '../Blocks';
 import { FileIcon } from '../FileIcon';
 import type { TableRecordKV } from './Table';
-import { type VerticalAlignment, getColumnAlignment } from './utils';
+import {
+    type VerticalAlignment,
+    getColumnAlignment,
+    isContentRef,
+    isDocumentTableImageRecord,
+    isStringArray,
+} from './utils';
 
 const alignmentMap: Record<'text-left' | 'text-center' | 'text-right', string> = {
     'text-left': '**:text-left text-left',
@@ -58,18 +63,28 @@ export async function RecordColumnValue<Tag extends React.ElementType = 'div'>(
         return null;
     }
 
+    // Because definition and value depends on column, we have to check typing in each case at runtime.
+    // Validation should have been done at the API level, but we can't know typing based on `definition.type`.
+    // OpenAPI types cannot really handle discriminated unions based on a dynamic key.
     switch (definition.type) {
-        case 'checkbox':
+        case 'checkbox': {
+            if (value === null || typeof value !== 'boolean') {
+                return null;
+            }
             return (
                 <Checkbox
                     className={tcls('w-5', 'h-5')}
-                    checked={value as boolean}
+                    checked={value}
                     disabled={true}
                     aria-labelledby={ariaLabelledBy}
                 />
             );
+        }
         case 'rating': {
-            const rating = value as number;
+            if (typeof value !== 'number') {
+                return null;
+            }
+            const rating = value;
             const max = definition.max;
 
             return (
@@ -108,15 +123,21 @@ export async function RecordColumnValue<Tag extends React.ElementType = 'div'>(
                 </Tag>
             );
         }
-        case 'number':
+        case 'number': {
+            if (typeof value !== 'number') {
+                return null;
+            }
             return (
                 <Tag
                     className={tcls('text-base', 'tabular-nums', 'tracking-tighter')}
                     aria-labelledby={ariaLabelledBy}
                 >{`${value}`}</Tag>
             );
+        }
         case 'text': {
-            // @ts-ignore
+            if (typeof value !== 'string') {
+                return null;
+            }
             const fragment = getNodeFragmentByName(block, value);
             if (!fragment) {
                 return <Tag className={tcls(['w-full', verticalAlignment])}>{''}</Tag>;
@@ -149,8 +170,11 @@ export async function RecordColumnValue<Tag extends React.ElementType = 'div'>(
             );
         }
         case 'files': {
+            if (!isStringArray(value)) {
+                return null;
+            }
             const files = await Promise.all(
-                (value as string[]).map((fileId) =>
+                value.map((fileId) =>
                     context.contentContext
                         ? resolveContentRef(
                               {
@@ -221,10 +245,12 @@ export async function RecordColumnValue<Tag extends React.ElementType = 'div'>(
             );
         }
         case 'content-ref': {
-            const contentRef = value ? (value as ContentRef) : null;
+            if (value === null || !isContentRef(value)) {
+                return null;
+            }
             const resolved =
-                contentRef && context.contentContext
-                    ? await resolveContentRef(contentRef, context.contentContext, {
+                value && context.contentContext
+                    ? await resolveContentRef(value, context.contentContext, {
                           resolveAnchorText: true,
                           iconStyle: ['mr-2', 'text-tint-subtle'],
                       })
@@ -239,11 +265,11 @@ export async function RecordColumnValue<Tag extends React.ElementType = 'div'>(
                         <StyledLink
                             href={resolved.href}
                             insights={
-                                contentRef
+                                value
                                     ? {
                                           type: 'link_click',
                                           link: {
-                                              target: contentRef,
+                                              target: value,
                                               position: SiteInsightsLinkPosition.Content,
                                           },
                                       }
@@ -257,8 +283,11 @@ export async function RecordColumnValue<Tag extends React.ElementType = 'div'>(
             );
         }
         case 'users': {
+            if (!isStringArray(value)) {
+                return null;
+            }
             const resolved = await Promise.all(
-                (value as string[]).map(async (userId) => {
+                value.map(async (userId) => {
                     const contentRef: ContentRefUser = {
                         kind: 'user',
                         user: userId,
@@ -295,10 +324,13 @@ export async function RecordColumnValue<Tag extends React.ElementType = 'div'>(
             );
         }
         case 'select': {
+            if (!isStringArray(value)) {
+                return null;
+            }
             return (
                 <Tag aria-labelledby={ariaLabelledBy}>
                     <span className={tcls('inline-flex', 'gap-2', 'flex-wrap')}>
-                        {(value as string[]).map((selectId) => {
+                        {value.map((selectId) => {
                             const option = definition.options.find(
                                 (option) => option.value === selectId
                             );
@@ -329,8 +361,15 @@ export async function RecordColumnValue<Tag extends React.ElementType = 'div'>(
             );
         }
         case 'image': {
+            if (!isDocumentTableImageRecord(value)) {
+                return null;
+            }
+
             const image = context.contentContext
-                ? await resolveContentRef(value as ContentRef, context.contentContext)
+                ? await resolveContentRef(
+                      'ref' in value ? value.ref : value,
+                      context.contentContext
+                  )
                 : null;
 
             if (!image) {

packages/gitbook/src/components/DocumentView/Table/utils.ts

@@ -123,3 +123,35 @@ export function getColumnVerticalAlignment(column: DocumentTableDefinition): Ver
 
     return 'self-center';
 }
+
+/**
+ * Check if a value is a ContentRef.
+ * @param ref The value to check.
+ * @returns True if the value is a ContentRef, false otherwise.
+ */
+export function isContentRef(ref?: DocumentTableRecord['values'][string]): ref is ContentRef {
+    return Boolean(ref && typeof ref === 'object' && 'kind' in ref);
+}
+
+/**
+ * Check if a value is an array of strings.
+ * @param value The value to check.
+ * @returns True if the value is an array of strings, false otherwise.
+ */
+export function isStringArray(value?: DocumentTableRecord['values'][string]): value is string[] {
+    return Array.isArray(value) && value.every((v) => typeof v === 'string');
+}
+
+/**
+ * Check if a value is a DocumentTableImageRecord.
+ * @param value The value to check.
+ * @returns True if the value is a DocumentTableImageRecord, false otherwise.
+ */
+export function isDocumentTableImageRecord(
+    value?: DocumentTableRecord['values'][string]
+): value is DocumentTableImageRecord {
+    if (isContentRef(value) && (value.kind === 'file' || value.kind === 'url')) {
+        return true;
+    }
+    return Boolean(value && typeof value === 'object' && 'ref' in value && isContentRef(value.ref));
+}