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

Input requested: Generated SVG icon components #490

Closed
wants to merge 3 commits into from
Closed

Conversation

lyzadanger
Copy link
Contributor

Hallo! We've been talking some about re-implementing how our icon component(s) work. The current goal is standalone icon components (e.g. one component for each icon).

This PR demonstrates one auto-generating approach which seems to be working fine except for a casing conflict between React and preact for SVG element attribute names. React camel-cases them; preact does not. The generating utility I used here (SVGR) is popping out React components and so the attribute casing is currently problematic.

I'm looking for input here on:

  • What do you think we should do about the casing issue?
  • Other than the casing issue, is there anything about the generated components that seems problematic?

Casing-fix possibilities

  • Use preact-compat. It works. But it's not what we want to do, and downstream projects would have to use preact-compat, too.
  • Manipulate generated icons by hand (and use the ignore-existing option when generating icons so that these changes don't get overwritten). @robertknight has expressed a preference for a script-able approach, however.
  • Write our own translation script for the casing, either as a babel plugin or...sky's probably the limit.
  • Use a different tool. Nothing I spotted seemed as drop-in. We could use a tool like svgo to generate optimized SVG elements but we'd still have to wrap them into components somehow.
  • Contribute to (or fork, which is less appetizing) the SVGR project to address the issue at its source. SVGR has a bunch of sub-packages; the relevant one here is hast-util-to-babel-ast which has a set of mappings for attributes.

Trying out this branch.

If you check out this branch and run make dev and visit http://localhost:4001/components-icons, you'll see something like:

image

This is mostly right but the line weights, etc., are wrong because of the casing issue.

Turning on preact-compat gives this result (which is correct):

image

@robertknight
Copy link
Member

Relevant upstream issue is gregberge/svgr#450. This was closed by a bot due to inactivity, but the author said they would be open to a solution.

@lyzadanger
Copy link
Contributor Author

@robertknight This is from some notes I took on a scratch branch:

Note to self during development: preact expects SVG element attributes to be kebab-cased, but SVGR's hast-util-to-babel-ast maps all SVG attribute names to camel-cased variants, which is what React expects — i.e. there is a preact incompatibility with the current hast-util-to-babel-ast utility in SVGR. I was able to intervene by hacking the getKey function to return t.jsxIdentifier(key) without doing any mapping or logic. I did this by installing @svgr/cli locally and then literally hand-manipulating the built index.js of hast-util-to-babel-ast in node_modules. Obviously this is not a solution. But I don't want to forget how I did it.

@lyzadanger lyzadanger mentioned this pull request Jul 15, 2022
7 tasks
@lyzadanger
Copy link
Contributor Author

Closing in lieu of #494

@lyzadanger lyzadanger closed this Jul 15, 2022
@lyzadanger lyzadanger deleted the spike-icons branch August 17, 2022 18:54
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.

None yet

2 participants