update(titleProp:true): title should fallback to svg's title

[sudkumar]

Summary

Fixes #310: title should fallback to SVG's title when titleProps is set to true and no title is provided

I have updated the @svgr/babel-plugin-svg-dynamic-title plugin to insert a conditional statement for rendering title of the SVG. So now, if the titleProp is set to true and the SVG has a title element, then the React component will have following conditional statement to render the title

<!-- icon.svg -->
<svg><title>Default Title</title></svg>

<svg>
- {title}
+ {title === undefined ? "Default Title" : title}
</svg>

If there is no title in the SVG, then there will be no change in the React component (same is existing).

<svg>
  <title>{title}</title>
</svg>

Test plan

I have update the relevant test case and docs for the implemented functionality.

P.S.: This is my first attempt with a babel plugin's source code :). So please bear with me.

[vercel]

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

[codecov]

Codecov Report

Merging #311 into master will increase coverage by 0.19%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #311      +/-   ##
==========================================
+ Coverage   86.38%   86.58%   +0.19%     
==========================================
  Files          30       30              
  Lines         470      477       +7     
  Branches      132      135       +3     
==========================================
+ Hits          406      413       +7     
  Misses         53       53              
  Partials       11       11

Impacted Files	Coverage Δ
...ckages/babel-plugin-svg-dynamic-title/src/index.js	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update af054fb...1dbbfdd. Read the comment docs.

[codecov]

Codecov Report

Merging #311 into master will increase coverage by 0.19%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #311      +/-   ##
==========================================
+ Coverage   86.38%   86.58%   +0.19%     
==========================================
  Files          30       30              
  Lines         470      477       +7     
  Branches      132      135       +3     
==========================================
+ Hits          406      413       +7     
  Misses         53       53              
  Partials       11       11

Impacted Files	Coverage Δ
...ckages/babel-plugin-svg-dynamic-title/src/index.js	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update af054fb...0c04915. Read the comment docs.

[gregberge]

One change, title is actually define, so you can check it directly using title === undefined instead of doing a typeof comparison. Except that everything looks OK to me!

[sudkumar]

Thank you @neoziro for having a look into it.

I'm kind of noobie to the babel plugin world. So I went to the babel docs to find a definition for creating this expression but failed. I don't know how create this expression this:

{type === undefined  ? "Default Title" : title}

Please suggest the related docs.

[gregberge]

You can use https://astexplorer.net/ to see what type are used, then try to find the correct method in babel types.

For example:

t.binaryExpression(
  '===',
  expression,
  t.identifier('undefined'),
)

It should do the trick ;)

[sudkumar]

On it. Thank you.

[sudkumar]

@neoziro Updated. Please have a look.

[gregberge]

It looks good to me, I will merge it after the tests have ran.

[sudkumar]

@neoziro That was quick. Thank you.

Just one question: Will you be able to release it any time soon ?

[gregberge]

Done https://github.com/smooth-code/svgr/releases/tag/v4.3.0

[sudkumar]

Thank you.

[sudkumar]

@neoziro I was testing it in an application bootstrapped with CRA and got some issues.

        const existingTitle = (existingTitleChildren || [])
          .map(c => c.value)
          .join();

So the value is not accessible in the case of JSXExpressionContainer.

So I visited https://astexplorer.net/ and found that we need to handle these cases too.

        const existingTitle = (existingTitleChildren || [])
          .map(c => {
            switch (c.type) {
              case 'JSXExpressionContainer':
                return c.expression.value;
              default:
                return c.value;
            }
          })
          .join();

Please help me out. What all cases can be there ? Or is there any better way to get the inner text as string.

[gregberge]

I think there are several cases:

<title>Hello</title>
<title>{'Hello'}</title>
<title></title>
<title />
/* nothing */

Please add them to tests!

[sudkumar]

Alright. I will add them and update.

[sudkumar]

What if we do this

<svg>
{title === undefined ? <title>{any existing children}</title> : <title>{title}</title>}
</svg>

Then we do not need to handle any case separately.