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

Add transformAttributes option to @svgr/core and hast-util-to-babel-ast #927

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

bensaufley
Copy link

Summary

Allows disabling of JSX Attribute transformation (camelCasing of kebab-case attributes), which benefits Preact users (who are otherwise supported by SVGR explicitly, by jsxRuntime: 'classic-preact').

Would resolve #450

Test plan

Added a test/snapshot within hast-util-to-babel-ast to test that the output is as expected. Confirmed that other tests and snapshots were unaffected.

Notes

This is purely additive, with a default value of true to keep existing behavior.

I considered using discernible Preact implementation to set this to false by default, because without preact/compat, SVGR does not currently work with Preact. So something like jsx.transformAttributes ?? (config.jsxRuntime === 'classic-preact' || config.jsxRuntimeImport?.source.startsWith('preact')) – but I think it's a bad idea to make a breaking change like that.

I originally passed a separate config object all through the HAST package, but then I realized that Helpers was already sort of being used for that purpose? So I just spread the config into it. Let me know if that's an unreasonable approach.

Copy link

vercel bot commented Nov 21, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
svgr ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 20, 2023 4:16pm

@bensaufley
Copy link
Author

TBH I have no idea if this was the best way to go about this; I'm not totally familiar with the codebase. But I figured a PR would be better than just pinging on a closed issue

@gregberge
Copy link
Owner

Hello @bensaufley, thanks for this, could you please fix the tests?

@bensaufley
Copy link
Author

@gregberge looks like it was just Prettier first time around—ironically I had undone some of that stuff because I assumed it was my local settings. But a note: I have added the ability to pass in a custom transformer function, very simple, just a (orig: string) => string function, just because it seemed like it made sense while I was here. I'm not sure how many use cases there are for it, but it seems like a no-brainer to me. LMK if you think it's a bad idea.

@bensaufley
Copy link
Author

@gregberge are these "Deploy canceled" things failures of some kind I need to fix? I don't think so but just want to confirm. Anything else I can be doing here?

@bensaufley
Copy link
Author

@gregberge happy new year! Any chance this can move forward?

@bensaufley
Copy link
Author

@gregberge anything I can do to help this get merged/released? This is the only reason we're importing preact/compat into our application and besides filesize, preact/compat has all sorts of side effects that aren't all positive.

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

Successfully merging this pull request may close these issues.

Option for preserving kebab-case attributes (for Preact)
2 participants