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

[Proposal] AST approach for HTML, Tailwind, BEM, etc #120

Open
davestewart opened this issue Jul 19, 2024 · 3 comments
Open

[Proposal] AST approach for HTML, Tailwind, BEM, etc #120

davestewart opened this issue Jul 19, 2024 · 3 comments

Comments

@davestewart
Copy link
Contributor

davestewart commented Jul 19, 2024

Hey @bernaferrari,

Whilst working on the Tailwind renderer and the variables stuff, I noticed quite a lot of duplication, and some situations where it would be difficult or cumbersome to re-process elements once rendered as strings.

For example:

  • rendering the main code and preview Tailwind code requires two runs
  • Tailwind preview uses RegExps to parse hardcoded classes (adds complications for handling variables)
  • some functions require access to global settings
  • getLocalVariables() is deprecated in favour of getLocalVariablesAsync() meaning Tailwind functions may need to be async at some point

I know you have an interm "alt-node" format, but I figured an additional interim AST format would work at least in the HTML domain, so the Figma element would be parsed to this standard format, then that format could be more-easily processed then manipulated, for example:

  • single pass for HTML, Tailwind and Tailwind Preview
  • Tailwind preview uses AST rather than RegExps to walk classes and variable ids
  • resolve variables async
  • use single AST to render main and preview code (tweaking settings for each, injecting resolved variable values)
  • add in additional custom attributes easily (i.e. [Feature Request] Properties on exported code #96)

A single renderer which walks the tree is also simpler than having each node render its HTML.

Potentially could even simplify platform implementation by:

  • rendering to Tailwind by default
  • for HTML, walk the tree and expand classes to styles (using the build-time generated classes)
  • for BEM, walk the HTML tree and build CSS using layer names and styles
const node = parseElement(selected)
const htmlNode = walkTree(node, classesToStyles)
const html = renderHtml(htmlNode)
const vars = walkTree(htmlNode, findVariables)
const bemNode = walkTree(htmlNode, makeBEM)
const colors = await renderVariables(vars)
renderBEM(bemNode, colors)

I had a bit of bash this morning and got an initial renderer up and running (have yet to integrate with existing platform code):

CleanShot 2024-07-19 at 15 53 38

Here's the AST:

{
  tag: 'div',
  name: 'Root',
  classes: [ 'foo', 'bar' ],
  children: [
    {
      tag: 'div',
      name: 'Container',
      classes: [ 'child' ],
      children: [
        { tag: 'div', name: 'Child 1', classes: [...] },
        { tag: 'div', name: 'Child 2', styles: {...} },
        { tag: 'input', name: 'Child 3', attrs: {...} },
        { tag: 'div', name: 'Child 4', attrs: {...} },
        { tag: 'div', name: 'Child 5', styles: {...} }
      ]
    }
  ]
}

And the HTML:

<div data-name="Root" className="foo bar">
  <div data-name="Container" className="child">
    <div data-name="Child 1" className="foo bar" />
    <div data-name="Child 2" style="baz: 1px; qux: 2px;" />
    <input data-name="Child 3" checked="true" />
    <div data-name="Child 4" title="Missing variable!" />
    <div data-name="Child 5" style="padding-left: 1px; padding-right: 1px;" />
  </div>
</div>

Anyway.

Another one for ideas to improve.

I think I can probably finish the variables stuff for now, but this would certainly be a more robust solution in the long term.

Will commit the code at some point and PR.

@davestewart davestewart changed the title [Proposal] v-node approach for HTML and Tailwind [Proposal] v-node approach for HTML, Tailwind, BEM, etc Jul 19, 2024
@davestewart davestewart changed the title [Proposal] v-node approach for HTML, Tailwind, BEM, etc [Proposal] AST approach for HTML, Tailwind, BEM, etc Jul 19, 2024
@mimshwright
Copy link
Contributor

I like it!

@bernaferrari
Copy link
Owner

I like it too!

@mimshwright
Copy link
Contributor

@bernaferrari you mentioned AST refactoring in another issue...
I think the current conversion code is a bit difficult to work with. The HTML and Tailwind builders seem to have a lot of overlapping code, and it also seems to be spread out across multiple files without much benefit. There also seems to be some inconsistency with passing around the settings that are necessary for doing the conversion vs sharing them as global variables. Many of the functions output strings (rather than something like a key/value pair for styles) meaning each function needs to consider whether it has JSX formatting. That's a lot of duplicated work spread all around.
So I think the AST idea makes a lot of sense. It's probably how I would want to build it from scratch.
At the same time, I question whether it would add much benefit at this point and seems like it could be a lot of work to redo all of that. The main reason to do it might be to make things easier to edit in the future.
Maybe it makes sense to try to completely remake this whole plugin with everything you've learned, but without all the history and legacy stuff and focus on only one language to start?

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

No branches or pull requests

3 participants