fix(babel-plugin-svg-dynamic-title): dont render empty title (#341)

Currently when no title is passed and titleProp is set to `true`, we are
rendering an empty title element which is causing some issues. So we
have added a conditional statement for rendering the title element only
if the title props is not falsy. If we have an existing title, that will
rendered by default. Passing the `null` values as the title will cause
also cause the title element for not render even if a default title
exists on the svg

Fixes #333

Author: sudkumar

packages/babel-plugin-svg-dynamic-title/src/index.js

@@ -11,22 +11,31 @@ const plugin = ({ types: t }) => ({
         return
       }
 
-      function createTitle(children = []) {
+      function createTitle(children = [], attributes = []) {
         return t.jsxElement(
-          t.jsxOpeningElement(t.jsxIdentifier('title'), []),
+          t.jsxOpeningElement(t.jsxIdentifier('title'), attributes),
           t.jsxClosingElement(t.jsxIdentifier('title')),
           children,
         )
       }
-      function getTitleElement(existingTitleChildren = []) {
+      function getTitleElement(existingTitle) {
         const titleExpression = t.identifier('title')
-        let titleElement = createTitle([
-          t.jsxExpressionContainer(titleExpression),
-        ])
-        if (existingTitleChildren && existingTitleChildren.length) {
+        let titleElement = t.conditionalExpression(
+          titleExpression,
+          createTitle(
+            [t.jsxExpressionContainer(titleExpression)],
+            existingTitle ? existingTitle.openingElement.attributes : [],
+          ),
+          t.nullLiteral(),
+        )
+        if (
+          existingTitle &&
+          existingTitle.children &&
+          existingTitle.children.length
+        ) {
           // if title already exists
           // render as follows
-          const fallbackTitleElement = createTitle(existingTitleChildren)
+          const fallbackTitleElement = existingTitle
           // {title === undefined ? fallbackTitleElement : titleElement}
           const conditionalExpressionForTitle = t.conditionalExpression(
             t.binaryExpression(
@@ -38,6 +47,8 @@ const plugin = ({ types: t }) => ({
             titleElement,
           )
           titleElement = t.jsxExpressionContainer(conditionalExpressionForTitle)
+        } else {
+          titleElement = t.jsxExpressionContainer(titleElement)
         }
         return titleElement
       }
@@ -49,7 +60,7 @@ const plugin = ({ types: t }) => ({
         if (!childPath.isJSXElement()) return false
         if (childPath.node === titleElement) return false
         if (childPath.node.openingElement.name.name !== 'title') return false
-        titleElement = getTitleElement(childPath.node.children)
+        titleElement = getTitleElement(childPath.node)
         childPath.replaceWith(titleElement)
         return true
       })

packages/babel-plugin-svg-dynamic-title/src/index.test.js

@@ -13,36 +13,44 @@ const testPlugin = (code, options) => {
 describe('plugin', () => {
   it('should add title attribute if not present', () => {
     expect(testPlugin('<svg></svg>')).toMatchInlineSnapshot(
-      `"<svg><title>{title}</title></svg>;"`,
+      `"<svg>{title ? <title>{title}</title> : null}</svg>;"`,
     )
   })
 
   it('should add title element and fallback to existing title', () => {
     // testing when the existing title contains a simple string
     expect(testPlugin(`<svg><title>Hello</title></svg>`)).toMatchInlineSnapshot(
-      `"<svg>{title === undefined ? <title>Hello</title> : <title>{title}</title>}</svg>;"`,
+      `"<svg>{title === undefined ? <title>Hello</title> : title ? <title>{title}</title> : null}</svg>;"`,
     )
     // testing when the existing title contains an JSXExpression
     expect(
       testPlugin(`<svg><title>{"Hello"}</title></svg>`),
     ).toMatchInlineSnapshot(
-      `"<svg>{title === undefined ? <title>{\\"Hello\\"}</title> : <title>{title}</title>}</svg>;"`,
+      `"<svg>{title === undefined ? <title>{\\"Hello\\"}</title> : title ? <title>{title}</title> : null}</svg>;"`,
+    )
+  })
+  it('should preserve any existing title attributes', () => {
+    // testing when the existing title contains a simple string
+    expect(
+      testPlugin(`<svg><title attr='a'>Hello</title></svg>`),
+    ).toMatchInlineSnapshot(
+      `"<svg>{title === undefined ? <title attr='a'>Hello</title> : title ? <title attr='a'>{title}</title> : null}</svg>;"`,
     )
   })
   it('should support empty title', () => {
     expect(testPlugin('<svg><title></title></svg>')).toMatchInlineSnapshot(
-      `"<svg><title>{title}</title></svg>;"`,
+      `"<svg>{title ? <title>{title}</title> : null}</svg>;"`,
     )
   })
   it('should support self closing title', () => {
     expect(testPlugin('<svg><title /></svg>')).toMatchInlineSnapshot(
-      `"<svg><title>{title}</title></svg>;"`,
+      `"<svg>{title ? <title>{title}</title> : null}</svg>;"`,
     )
   })
 
   it('should work if an attribute is already present', () => {
     expect(testPlugin('<svg><foo /></svg>')).toMatchInlineSnapshot(
-      `"<svg><title>{title}</title><foo /></svg>;"`,
+      `"<svg>{title ? <title>{title}</title> : null}<foo /></svg>;"`,
     )
   })
 })

packages/babel-preset/src/index.test.js

@@ -63,7 +63,7 @@ describe('preset', () => {
                   
                   const SvgComponent = ({
                     title
-                  }) => <svg><title>{title}</title></svg>;
+                  }) => <svg>{title ? <title>{title}</title> : null}</svg>;
                   
                   export default SvgComponent;"
             `)
@@ -82,7 +82,7 @@ describe('preset', () => {
                   
                   const SvgComponent = ({
                     title
-                  }) => <svg>{title === undefined ? <title>Hello</title> : <title>{title}</title>}</svg>;
+                  }) => <svg>{title === undefined ? <title>Hello</title> : title ? <title>{title}</title> : null}</svg>;
                   
                   export default SvgComponent;"
             `)
@@ -99,7 +99,7 @@ describe('preset', () => {
                   
                   const SvgComponent = ({
                     title
-                  }) => <svg>{title === undefined ? <title>{\\"Hello\\"}</title> : <title>{title}</title>}</svg>;
+                  }) => <svg>{title === undefined ? <title>{\\"Hello\\"}</title> : title ? <title>{title}</title> : null}</svg>;
                   
                   export default SvgComponent;"
             `)

packages/cli/src/__snapshots__/index.test.js.snap

@@ -320,7 +320,7 @@ exports[`cli should support various args: --title-prop 1`] = `
 
 const SvgFile = ({ title, ...props }) => (
   <svg width={48} height={1} {...props}>
-    <title>{title}</title>
+    {title ? <title>{title}</title> : null}
     <path d=\\"M0 0h48v1H0z\\" fill=\\"#063855\\" fillRule=\\"evenodd\\" />
   </svg>
 )

packages/core/src/__snapshots__/convert.test.js.snap

@@ -332,7 +332,7 @@ exports[`convert config should support options { titleProp: true } 1`] = `
 
 const SvgComponent = ({ title, ...props }) => (
   <svg width={88} height={88} {...props}>
-    <title>{title}</title>
+    {title ? <title>{title}</title> : null}
     <g
       stroke=\\"#063855\\"
       strokeWidth={2}
@@ -361,7 +361,7 @@ const SvgComponent = ({ title, ...props }) => (
     }}
     {...props}
   >
-    <title>{title}</title>
+    {title ? <title>{title}</title> : null}
     <path d=\\"M0 0h24v24H0z\\" fill=\\"none\\" />
   </svg>
 )