Add option to explicitly use named export

[rayrw]

Summary

related: #557, kazijawad/esbuild-plugin-svgr#2

By default, when @svgr/webpack is the only loader handling svg files, it will always export the SVG React Component via default export. However, there may be cases where we prefer always exporting it via named export.

In light of this, I'd like to propose adding a forceNamedExport exportType (#584 (review)) config option to explicitly export the React Component via named export.

Example

In Webpack config:

{
  test: /\.svg$/,
  use: [
    {
      loader: '@svgr/webpack',
      options: {
        exportType: 'named',
      },
    },
  ],
}

This will allow us in React side:

import { ReactComponent as Star } from './star.svg'

const App = () => (
  <div>
    <Star />
  </div>
)

Test plan

added test cases and docs

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/gregberge/svgr/Eerrm7dBYatRT3ja26EtxmrPVQXW
✅ Preview: https://svgr-git-fork-rayrw-add-force-named-export-option-gregberge.vercel.app

[codecov]

Codecov Report

Merging #584 (f42c705) into main (1058d91) will decrease coverage by 4.18%.
The diff coverage is 100.00%.

❗ Current head f42c705 differs from pull request most recent head 61357ea. Consider uploading reports for the commit 61357ea to get more accurate results

@@            Coverage Diff             @@
##             main     #584      +/-   ##
==========================================
- Coverage   91.38%   87.19%   -4.19%     
==========================================
  Files          31       31              
  Lines        1079      570     -509     
  Branches      324      160     -164     
==========================================
- Hits          986      497     -489     
+ Misses         88       58      -30     
- Partials        5       15      +10     

Impacted Files	Coverage Δ
packages/core/src/config.js	100.00% <ø> (+7.84%)	⬆️
...s/babel-plugin-transform-svg-component/src/util.js	100.00% <100.00%> (+5.36%)	⬆️
.../hast-util-to-babel-ast/src/stringToObjectStyle.js	0.00% <0.00%> (-67.40%)	⬇️
packages/plugin-svgo/src/index.js	66.66% <0.00%> (-33.34%)	⬇️
packages/plugin-svgo/src/config.js	76.19% <0.00%> (-23.81%)	⬇️
...el-plugin-replace-jsx-attribute-value/src/index.js	68.75% <0.00%> (-19.49%)	⬇️
packages/hast-util-to-babel-ast/src/one.js	71.42% <0.00%> (-16.08%)	⬇️
packages/hast-util-to-babel-ast/src/handlers.js	69.56% <0.00%> (-15.86%)	⬇️
...ckages/babel-plugin-svg-em-dimensions/src/index.js	93.75% <0.00%> (-6.25%)	⬇️
packages/babel-preset/src/index.js	93.54% <0.00%> (-5.01%)	⬇️
... and 25 more

---

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 1058d91...61357ea. Read the comment docs.

[fgblomqvist]

I am using rollup and I was also looking for something like this. However, may I suggest a slightly different approach?

Instead of adding forceNamedExport, you could expand the type of namedExport to be boolean | string and then do a check like "if namedExport === true, use default name or if typeof namedExport === 'string', use namedExport, else do default export.

Would perhaps result in a better UX (I for one find it odd that I can specify namedExport: 'foo' without it doing anything).

[fgblomqvist]

Any thoughts on this @gregberge?

[gregberge]

I am thinking about using exportType instead of forceNamedExport. The option is more explicit.

[gregberge]

The check of opts.namedExport is not needed I think.

[gregberge]

Not relative to rollup isn't it?

[gregberge]

A paragraph to explain exportType option please.

[rayrw]

@fgblomqvist Thanks for your suggestion!
I also first came up with a similar idea. However, since the default value of namedExport has already been set to ReactComponent, we might have to either change the default behavior to always exporting it via named export, or change the default value of namedExport to false which means always exporting via default export.

Any of these options will drop the current default behavior which attempts to export it via default export if there's no any other loader/plugin using default export. Therefore, I'm afarid proceeding with this idea will introduce a huge breaking change. We could introduce an option of setting namedExport to undefined to fallback to the current behavior, but I'm afarid it sounds confusing.

Because of this, I agree with the suggestion from @gregberge as a safer and more backward compatible option.
To reduce the confusion of having both exportType and namedExport options, I've updated the documentation to put a paragraph near the explanation of namedExport option to explain the default behavior and the usage of the new exportType option. I hope this sounds reasonable.

[fgblomqvist]

@rayrw yeah sounds good to me, thanks for the lengthy explanation 🙂

[gregberge]

Thanks!

[fgblomqvist]

Would it be possible to get a release cut with this?

[gregberge]

@fgblomqvist you will have to wait a little bit longer. I will release a major and I have a stuff to do before releasing :)