Repository: speee/jsx-slack
Pull request #64 (2019-10-01 04:16:30)

Check invalid elements in <Blocks> and <Input> strictly
By adding input-compatible props to existed interactive components (#63), the possibility of wrong usage would increase. For example, using `label` prop at interactive components in `<Blocks>` and `<Actions>` would not be thrown error but outputs invalid JSON when sending to Slack.

For helping user to notice mistake, we've refactored JSX parse logic and added after-parse validation to `<Blocks>` and `<Input>`.

Especially these were hard to check rendered elements if is used multi-level `<Fragment>`, so we changed internal JSX object to allow defining event function called at after parsed as internal props. Thereby we can validate to the final output of JSON.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 741a56d..d2ef513 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -13,6 +13,7 @@
 ### Changed
 
 - Bump dependent packages to the latest version ([#59](https://github.com/speee/jsx-slack/pull/59))
+- Check invalid elements in `<Blocks>` and `<Input>` strictly ([#64](https://github.com/speee/jsx-slack/pull/64))
 
 ### Deprecated
 
diff --git a/src/block-kit/Blocks.tsx b/src/block-kit/Blocks.tsx
index f2bffe1..dcb137c 100644
--- a/src/block-kit/Blocks.tsx
+++ b/src/block-kit/Blocks.tsx
@@ -1,5 +1,5 @@
 /** @jsx JSXSlack.h */
-import { JSXSlack } from '../jsx'
+import { JSXSlack, jsxOnParsed } from '../jsx'
 import { ArrayOutput, isNode, wrap } from '../utils'
 import { Divider, Image, Section } from './index'
 
@@ -18,35 +18,52 @@ export enum InternalBlockType {
 
 export const blockTypeSymbol = Symbol('jsx-slack-block-type')
 
+const knownBlocks = [
+  'actions',
+  'context',
+  'divider',
+  'file',
+  'image',
+  'input',
+  'section',
+]
+
 export const Blocks: JSXSlack.FC<BlocksProps> = props => {
   const internalType: InternalBlockType | undefined = props[blockTypeSymbol]
 
   const normalized = wrap(props.children).map(child => {
-    if (child && isNode(child)) {
-      if (typeof child.type === 'string') {
-        // Aliasing intrinsic elements to Block component
-        switch (child.type) {
-          case 'hr':
-            return <Divider {...child.props}>{...child.children}</Divider>
-          case 'img':
-            return <Image {...child.props}>{...child.children}</Image>
-          case 'section':
-            return <Section {...child.props}>{...child.children}</Section>
-          default:
-            throw new Error('<Blocks> allows only including Block component.')
-        }
-      } else if (child.type === JSXSlack.NodeType.object) {
-        // Check layout blocks
-        if (internalType === undefined && child.props.type === 'input')
+    if (child && isNode(child) && typeof child.type === 'string') {
+      // Aliasing intrinsic elements to Block component
+      switch (child.type) {
+        case 'hr':
+          return <Divider {...child.props}>{...child.children}</Divider>
+        case 'img':
+          return <Image {...child.props}>{...child.children}</Image>
+        case 'section':
+          return <Section {...child.props}>{...child.children}</Section>
+        default:
           throw new Error(
-            '<Input> block cannot place in <Blocks> container for messaging.'
+            '<Blocks> allows only including layout block components.'
           )
       }
     }
     return child
   })
 
-  return <ArrayOutput>{normalized}</ArrayOutput>
+  const node = <ArrayOutput>{normalized}</ArrayOutput>
+
+  node.props[jsxOnParsed] = parsed => {
+    // Check the final output again
+    if (parsed.some(b => !knownBlocks.includes(b.type)))
+      throw new Error('<Blocks> allows only including layout block components.')
+
+    if (internalType === undefined && parsed.some(b => b.type === 'input'))
+      throw new Error(
+        '<Input> block cannot place in <Blocks> container for messaging.'
+      )
+  }
+
+  return node
 }
 
 export const BlocksInternal: JSXSlack.FC<
diff --git a/src/block-kit/Input.tsx b/src/block-kit/Input.tsx
index 286538a..a4ef933 100644
--- a/src/block-kit/Input.tsx
+++ b/src/block-kit/Input.tsx
@@ -1,6 +1,6 @@
 /** @jsx JSXSlack.h */
 import { InputBlock } from '@slack/types'
-import { JSXSlack } from '../jsx'
+import { JSXSlack, jsxOnParsed } from '../jsx'
 import { ObjectOutput, coerceToInteger } from '../utils'
 import { BlockComponentProps } from './Blocks'
 import { plainText } from './composition/utils'
@@ -43,6 +43,21 @@ export type WithInputProps<T> =
   | T & { [key in keyof InputCommonProps]?: undefined }
   | T & InputCommonProps
 
+const knownInputs = [
+  'channels_select',
+  'conversations_select',
+  'datepicker',
+  'external_select',
+  'multi_channels_select',
+  'multi_conversations_select',
+  'multi_external_select',
+  'multi_static_select',
+  'multi_users_select',
+  'plain_text_input',
+  'static_select',
+  'users_select',
+]
+
 export const wrapInInput = (
   element: JSXSlack.Node<any>,
   props: InputCommonProps
@@ -77,8 +92,7 @@ export const Input: JSXSlack.FC<InputProps> = props => {
   if (props.children === undefined) return InputComponent(props)
 
   const hintText = props.hint || props.title
-
-  return (
+  const node = (
     <ObjectOutput<InputBlock>
       type="input"
       block_id={props.id || props.blockId}
@@ -88,6 +102,14 @@ export const Input: JSXSlack.FC<InputProps> = props => {
       element={JSXSlack(props.children)}
     />
   )
+
+  node.props[jsxOnParsed] = parsed => {
+    // Check the final output
+    if (!(parsed.element && knownInputs.includes(parsed.element.type)))
+      throw new Error('A wrapped element in <Input> is invalid.')
+  }
+
+  return node
 }
 
 export const Textarea: JSXSlack.FC<TextareaProps> = props =>
diff --git a/src/jsx.ts b/src/jsx.ts
index 5214d8a..f15de8a 100644
--- a/src/jsx.ts
+++ b/src/jsx.ts
@@ -6,6 +6,8 @@ import { wrap } from './utils'
 
 let internalExactMode = false
 
+type OnParsed = (parsed: any, context: ParseContext) => void
+
 enum ParseMode {
   normal,
   plainText,
@@ -18,13 +20,16 @@ export interface ParseContext {
   mode: ParseMode
 }
 
-export function JSXSlack(
-  node: JSXSlack.Node,
-  context: ParseContext = {
-    builts: [],
-    elements: [],
-    mode: ParseMode.normal,
-  }
+export const jsxOnParsed = Symbol('jsx-slack-jsxOnParsed')
+
+export function JSXSlack(node: JSXSlack.Node) {
+  // eslint-disable-next-line @typescript-eslint/no-use-before-define
+  return parseJSX(node, { builts: [], elements: [], mode: ParseMode.normal })
+}
+
+function parseJSX(
+  node: JSXSlack.Node & { [jsxOnParsed]?: OnParsed },
+  context: ParseContext
 ) {
   const children = JSXSlack.normalizeChildren(
     node.props.children || node.children || []
@@ -38,41 +43,48 @@ export function JSXSlack(
       (arr, c) => {
         const ctx = { ...nextCtx, builts: arr }
         const ret =
-          typeof c === 'string' ? processString(c, ctx) : JSXSlack(c, ctx)
+          typeof c === 'string' ? processString(c, ctx) : parseJSX(c, ctx)
 
         return [...ctx.builts, ...(ret ? [ret] : [])]
       },
       [] as any[]
     )
 
-  switch (node.type) {
-    case JSXSlack.NodeType.object: {
-      const obj = { ...node.props }
-
-      Object.keys(obj).forEach(key => obj[key] === undefined && delete obj[key])
-      return obj
-    }
-    case JSXSlack.NodeType.array:
-      return toArray()
-    case JSXSlack.NodeType.html:
-      return turndown(toArray({ ...context, mode: ParseMode.HTML }).join(''))
-    case JSXSlack.NodeType.escapeInHtml:
-      return escapeChars(toArray().join(''))
-    case JSXSlack.NodeType.string:
-      return toArray({ ...context, mode: ParseMode.plainText }).join('')
-    default:
-      if (typeof node.type === 'string') {
-        context.elements.push(node.type)
-
-        try {
-          if (context.mode === ParseMode.plainText) return toArray()
-          return parse(node.type, node.props, toArray(), context)
-        } finally {
-          context.elements.pop()
+  const ret = (() => {
+    switch (node.type) {
+      case JSXSlack.NodeType.object:
+        return Object.keys(node.props).reduce(
+          (obj, k) =>
+            node.props[k] !== undefined ? { ...obj, [k]: node.props[k] } : obj,
+          {}
+        )
+      case JSXSlack.NodeType.array:
+        return toArray()
+      case JSXSlack.NodeType.html:
+        return turndown(toArray({ ...context, mode: ParseMode.HTML }).join(''))
+      case JSXSlack.NodeType.escapeInHtml:
+        return escapeChars(toArray().join(''))
+      case JSXSlack.NodeType.string:
+        return toArray({ ...context, mode: ParseMode.plainText }).join('')
+      default:
+        if (typeof node.type === 'string') {
+          context.elements.push(node.type)
+
+          try {
+            if (context.mode === ParseMode.plainText) return toArray()
+            return parse(node.type, node.props, toArray(), context)
+          } finally {
+            context.elements.pop()
+          }
         }
-      }
-      throw new Error(`Unknown node type: ${node.type}`)
-  }
+        throw new Error(`Unknown node type: ${node.type}`)
+    }
+  })()
+
+  if (node.props[jsxOnParsed] !== undefined)
+    node.props[jsxOnParsed](ret, context)
+
+  return ret
 }
 
 // eslint-disable-next-line no-redeclare


Test patch:
diff --git a/test/index.tsx b/test/index.tsx
index 204b9a1..7829b6a 100644
--- a/test/index.tsx
+++ b/test/index.tsx
@@ -38,10 +38,30 @@ import JSXSlack, {
   Textarea,
   UsersSelect,
 } from '../src/index'
+import { jsxOnParsed } from '../src/jsx'
 
 beforeEach(() => JSXSlack.exactMode(false))
 
 describe('jsx-slack', () => {
+  describe('#JSXSlack', () => {
+    it('executes specified func after parsing if passed internal node has jsxOnParsed symbol prop', () => {
+      const onParsed = jest.fn()
+      const node = (
+        <SelectFragment>
+          <Option value="a">A</Option>
+        </SelectFragment>
+      )
+
+      node.props[jsxOnParsed] = onParsed
+      JSXSlack(node)
+
+      expect(onParsed).toBeCalledTimes(1)
+    })
+
+    it('throws error by passed invalid node', () =>
+      expect(() => JSXSlack({ props: {}, type: -1 } as any)).toThrow())
+  })
+
   describe('Container components', () => {
     describe('<Blocks>', () => {
       it('throws error when <Blocks> has unexpected element', () => {
@@ -53,6 +73,14 @@ describe('jsx-slack', () => {
           )
         ).toThrow()
 
+        expect(() =>
+          JSXSlack(
+            <Blocks>
+              <Escape>unexpected</Escape>
+            </Blocks>
+          )
+        ).toThrow()
+
         // <Input> block cannot use in message
         expect(() =>
           JSXSlack(
@@ -1056,6 +1084,21 @@ describe('jsx-slack', () => {
             ).blocks
           ).toStrictEqual(blocks)
         })
+
+        it('throws error when wrapped invalid element', () => {
+          expect(() =>
+            JSXSlack(
+              <Modal title="test">
+                <Input label="invalid">
+                  <Overflow actionId="overflow">
+                    <OverflowItem value="a">A</OverflowItem>
+                    <OverflowItem value="b">B</OverflowItem>
+                  </Overflow>
+                </Input>
+              </Modal>
+            )
+          ).toThrow(/invalid/)
+        })
       })
 
       describe('as block element for plain-text input', () => {