titleProp with existing title not working properly

[sudkumar]

🐛 Bug Report

In the last release 4.3.0, we introduced a feature for falling back to existing title when titleProp is set to true. But the feature is not working as expected in an application. It doesn't fallback to existing title.

To Reproduce

1. Fork create-react-app and update the @svgr/webpack package in packages/react-scripts to 4.3.0 and update packages/react-scripts/config/webpack.config.js to include the +titleProp option in svgr loader.
2. Run yarn to install all the dependencies
3. Commit your changes if necessary
4. Create an application with this create-react-app setup by running yarn create-react-app my-app
5. Now go into the my-app folder and change the logo.svg to add a title

<svg...>
+ <title>React Logo</title>

6. Import this svg as ReactComponent in App.js

- import logo  from "./logo.svg"
+ import { ReactComponent as Logo } from "./logo.svg"

function App () {
  return (
    <div className="App">
      <header className="App-header">
-         <img src={logo} className="App-logo" alt="logo" />
+         <Logo className="App-logo" />

7. Now run the application with yarn start and inspect the title element in the developer tools. We see an empty title element (<title></title>).

Expected behavior

I expect the title element to be <title>React Logo</title>

Possible Issues

Looking at the source code of packages/babel-plugin-svg-dynamic-title/src/index.js, we see that the children of the title element are joined with the .value which works only for JSXText but not for JSXExpressionContainer. And it seems like in the application I tested, it was returning the title's children as JSXExpressionContainer.

Possible Solution

Instead returning the title expression from conditional check ({title === undefined ? 'Fallback_Title_Children' : title}), we can return the title element itself ({title === undefined ? <title>{Fallback_Title_Children}</title> : <title>{title}</title>}). That way, we do not need to handle any cases that may occur for the children of existing title.

[sudkumar]

@neoziro What are your thoughts on this ?

[gregberge]

@sudkumar I commented on your PR.