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

Update svgr codemod to output TS for tsx inputs #8935

Merged
merged 2 commits into from
Jul 19, 2023

Conversation

Tobbe
Copy link
Member

@Tobbe Tobbe commented Jul 18, 2023

If an svg is imported in a .tsx file the user probably want the generated SVG component file to be using TypeScript. That's what this PR does.

@Tobbe Tobbe added the release:fix This PR is a fix label Jul 18, 2023
@Tobbe Tobbe added this to the v6.0.0 milestone Jul 18, 2023
@@ -17,6 +26,7 @@ async function convertSvgToReactComponent(
{
jsxRuntime: 'automatic',
plugins: ['@svgr/plugin-jsx'],
typescript,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This, together with the const isTS = code below is the main change for this PR.
The rest is mostly changes to make the code easier to read and understand

@@ -111,15 +122,20 @@ export default async function transform(file: FileInfo, api: API) {

// The absolute path to the new file
const outputPath = path.join(
path.dirname(filePath),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading this code in isolation it was hard to know what filePath was. Path to the file we're parsing? Path to the svg we found in that file? Path to the file we're going to generate?

This change hopefully makes it more clear that it's the path to the svg file the code is importing

const root = j(file.source)

// Do this lazily
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cost of importing getPaths has already been paid by replaceComponentSvgs.yargs.ts, so moved this to a regular import, so you don't have to wonder why we're doing a lazy import.

@@ -25,16 +35,16 @@ async function convertSvgToReactComponent(

await fs.writeFile(outputPath, jsCode)

console.log()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra blank line needed for Listr to not mess up the output

@Tobbe Tobbe requested a review from jtoar July 18, 2023 23:17
Copy link
Contributor

@jtoar jtoar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worked great for me, thanks @Tobbe!

@jtoar jtoar merged commit 7f4dbca into redwoodjs:main Jul 19, 2023
@Tobbe Tobbe deleted the tobbe-svgr-tsx branch July 19, 2023 06:08
jtoar pushed a commit that referenced this pull request Jul 19, 2023
If an svg is imported in a .tsx file the user probably want the
generated SVG component file to be using TypeScript. That's what this PR
does.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:fix This PR is a fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants