Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the Svg prefix globally #667

Closed
advename opened this issue Jan 19, 2022 · 3 comments
Closed

Use the Svg prefix globally #667

advename opened this issue Jan 19, 2022 · 3 comments

Comments

@advename
Copy link

advename commented Jan 19, 2022

🚀 Feature Proposal

Keep it consistent and use the Svg prefix globally for:

  • the file name, e.g. smiley.svg -> SvgSmiley.jsx
  • the component name, e.g. SvgSmiley() (already implemented)
  • the index.js exports, e.g. instead of export { default as Smiley } from './Smiley';, do export { default as SvgSmiley } from './Smiley';

Motivation

Regarding #190, all Component names should be prefixed with Svg.
#383 mentions that the index.jsonly exports with Svg prefix if the component name starts with a number.
I know that it is possible to alias an import, but it's confusing for a new user to understand when/or when not the prefix is used.
End-users of this tool might have to run additional processing. First, after many trials and errors have I noticed the inconsistency.

Monkey Patch

Currently, I have to use a custom index template, e.g.

// svgr.config.js

const path = require('path');

/**
 * Custom index.js template
 */
function defaultIndexTemplate(filePaths) {
  const exportEntries = filePaths.map((filePath) => {
    const basename = path.basename(filePath, path.extname(filePath));
    const exportName = /^\d/.test(basename) ? `Svg${basename}` : basename;
    return `export { default as Svg${exportName} } from './${basename}'`;
  });
  return exportEntries.join('\n');
}

module.exports = {
  indexTemplate: defaultIndexTemplate,
};

and run a command to rename all generated component files with an Svg prefix.

@gregberge
Copy link
Owner

Hello @advename, I understand your misunderstood and I will try to clear it up.

The component name is SvgXXX

Yes we only talk about the component name here, not the export name.

  • By default, it uses the default export, so not used.
  • With the option exportType: named it uses ReactComponent

SvgXXX is never exported, it is the name of the component visible in React developer tools when there is no minification.

index.js exports SvgXXX

Yes only when the filename starts with a number. Just because we do not have a choice here, JavaScript does not allow export name starting with a number. Note starting a SVG file with a number is kind of weird, but SVGR handles all use-cases, so.

I will not prefix everything with SvgXXX for a rare case when the filename starts with a number, it is easier for users to have the name of the SVG inferred from the filename without any prefix.


To resume, things are confusing for a new user only if it has filenames starting with a number, in this case yes it is.

The implementation is SVGR is coherent, I don't want to change it. The "Monkey Patch" you proposed is not a "Monkey Patch" it is just a custom configuration, nothing more. Feel free to use it, it is perfectly fine 👍. The other option is to rename all your SVG's starting with a number with "svg-" and it will not be confusing any more 😉.

@advename
Copy link
Author

One of my main point, beside keeping it coherent with the component name, is regarding your comment here in #190.

As discussed in #168, the component name generated could be a problem with some filename. For an example, "infinity.js" will generate "Infinity" that is a reserved keyword.

The simplest solution is to prefix all component name with Svg. We already do it for "number" filenames.

I.e., SVGR internally avoids "infinity" component name with prefixing it with Svg. But then, by exporting it without the Svg prefix means the issue has just been "forwarded" to the consuming end-user, who then may decide to import {Infinity} from "./icons" and cross fingers that it won't break anything, or do import {Infinity as SvgInfinity} from "./icons".

IMHO, forcing end-users to import {SvgInfinity} from "./icons" prevents unknowledgeable users breaking their codebase.

@gregberge
Copy link
Owner

@advename I understand but there is a workaround to not breaking the code, so it is not a problem. Everything generated should be valid, but the usage is the responsibility of the user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants