mikaelvesavuori / figmagic

Figmagic is the missing piece between DevOps and design: Generate design tokens, export graphics, and extract design token-driven React components from your Figma documents.
https://docs.figmagic.com
MIT License
807 stars 71 forks source link

Element generation: Imported JavaScript for elements becomes invalid if using dashes #149

Closed glouhaichi closed 2 years ago

glouhaichi commented 2 years ago

When trying to import our elements from the Figma file, the JavaScript follows our naming convention to the letter, which cause the JavaScript code inside to be invalid.

import styled from 'styled-components';

import $ixc-cognira-cta-buttonCss from './$ixc-cognira-cta-buttonCss';

// Extend the below as needed
const $ixc-cognira-cta-buttonStyled = styled.div`
  ${$ixc-cognira-cta-buttonCss};
`;

export default $ixc-cognira-cta-buttonStyled;

As you can see, the variable names are following the naming convention that uses a kebab case, which represents invalid syntax.

Note: I tried altering the camelizeTokenNames setting but it seems to have no effect, void on playing with output formats as well.

mikaelvesavuori commented 2 years ago

Hey! This goes back to our previous conversation where I noted that the dollar sign does not seem like a valid use. I am surprised also if my implementation allowed the dollar sign, for that specific reason. From what I can see here, it is therefore not a Figmagic issue.

glouhaichi commented 2 years ago

@mikaelvesavuori does that mean that removing the dollar sign should fix the issue? Because the issue described in the JavaScript file is not the dollar sign per se, it's the fact that dashes are not allowed in variable names.

mikaelvesavuori commented 2 years ago

Maybe. I was speculating; this is one of those things that "should be" obvious, but raises questions that can easily be verified.

  1. The dollar sign did actually work though, again, seems like a very non-syntactic thing.

  2. I didn't even think of the fact that dashes in variable names aren't allowed... Stupid me.

The current implementation (I can't find it right now, at work) does base itself on some of the other names, so we are seeing unexpected, cascading errors because of the dashes, yes. My tests did not cover this scenario.

I will rename the issue slightly, to address this fact, in regards to element generation. This may be difficult because of matching logic and having several places where the matching/naming logic does not work the same.

mikaelvesavuori commented 2 years ago

Did a small check and I'm happy to say that the code was well-written enough to not pose a major impediment. Had to write yet another string sanitizer (this time allowing slashes, so we don't break folders support), but I just wanted to check with you if this would be more like what you are looking for—

Because of the dashes (etc), for file imports and so on, it would be necessary to have some strictures. One seemingly working solution would be do something like this, where a button in Figma, called $my-design-system-buttonwould simply be mydesignsystembutton on disk (having removed everything that is not A-Z, a-z, 0-9, and slashes). It does not do conversions or other stuff.

Screenshot 2021-11-08 at 20 48 29
glouhaichi commented 2 years ago

@mikaelvesavuori excellent, as long as it's valid JavaScript (or TypeScript) we would be good to go.

And if I'm allowed one small nitpick, I would suggest that the string santizer would transform $my-design-system-button into MyDesignSystemButton. It would fit nicely with the capitalized suffix Styled, plus it's actually a component and capitalization is standard for those.

Thank you.

mikaelvesavuori commented 2 years ago

This is solved, with PascalCase names, in 4.4.0.