FortAwesome / ember-fontawesome

Font Awesome Ember component using SVG with JS
https://fontawesome.com
MIT License
119 stars 49 forks source link

Design discussion about build time AST transform functionality #14

Open mlwilkerson opened 6 years ago

mlwilkerson commented 6 years ago

Opening a new issue here to carry forward design discussion about the AST transform functionality that @jrjohnson is porting from ember-font-awesome and mentioned in #8.


So, @jrjohnson how might these build time AST transforms impact animated icon use cases, or any case where the <svg> DOM tree changes in response to something happening in the app?

For example, with the "magic slider" example currently in the dummy app, the DOM tree under the <svg> changes structurally when you move that slider. So that seems like a case that couldn't be rendered statically at build time. Admittedly, most icons are probably static and could be rendered statically without any functional impact.

jrjohnson commented 6 years ago

There are two parts to this. For those properties that are part of the <svg> element the AST transform actually outputs something like a component that still has bound properties for things like animation. When those properties are dynamic like the "magic slider" they will remain dynamic in the final result.

The second part is more complicated and where I'm still a bit stuck. The children of the <svg> can't really be handled in this same way. For something like {{fa-icon iconName}} there is no way to dynamically lookup and change the <path> so, as far as I can tell it wont be possible to handle this case at all.

mlwilkerson commented 6 years ago

I'm not sure I follow, but just for the sake of clarity, and to make sure we're not overlooking something, here's what I see happening in the DOM with the magic slider:

When the slider is at 0, then magic=0, and thus there's no transform on the fontawesome icon object, and thus the elements rendered have the structure:

<svg>
  <path></path>
</svg>

If the slider is non-zero, then that kicks in the transform attribute on the fa-icon component, and the rendered result has a structure of:

<svg>
  <g>
    <g>
      <path></path>
    </g>
  </g>
</svg>

And of course, some of the attributes on some of these elements are also changing as the slider slides.

I can imagine that the AST transform could output a structure like either the former or the latter, and within those structures, bind some attributes that could allow it to be dynamic somehow. But it wouldn't know to output both structures, right? So I think that's an example of your "second part".

However, in the case of the spinning magic (and anything like it), the structure changes depending on that bound value. And that's true not only where you have {{fa-icon iconName}} (i.e. where the value of the icon can't be statically analyzed), but also in this case where the icon value is specified statically as a string. It's the changing transform that causes a different DOM structure to be emitted by the fontawesome API. The spinning magic looks like this: {{fa-icon icon='magic' transform=(concat 'rotate-' magic)}} where magic is bound to the value of the slider input.

So I wonder if there's some way that the AST transform can either guarantee that it's not eliminating some intended dynamic behavior, or else warn if it can't guarantee that. And then I guess there'd need to be some documentation about when that AST transform optimization is valid, and when it's not. For example, if it sees that the component is binding to the transform attribute, all bets are off, as the magic slider demonstrates.

Does it sound like I'm in the same neighborhood of thinking as you here?

jrjohnson commented 6 years ago

Yes, we're on the same page. We're only going to be able to transform some of the simplest permutations. In fact I wasn't aware of the nested <g> situation with some transforms. So I'm going to need to sit down again and just output a bunch of icons and see what they look like. Even if we can only handle {{fa-icon 'user'}} there is still a lot of value in that since it is such a common use case and I hope we can build on that foundation to add more possibilities.

mlwilkerson commented 6 years ago

When you get to outputting icons to see what they look like—if you find something that's a distinctly different class—I think it would help for us to get a representative example into the dummy app and/or test suite. That was part of what I had in mind when I made that dummy app template: acceptance criteria, if you will. But I did not do a systematic combing through all possibilities, so it's quite possible that there are others.

In particular, the example with the mask and the example with the magic slider are two that I've visually inspected time and again throughout development as a way to make sure I'm not breaking something with my commits.

@robmadole may know off the top of his head of any other classes of examples that we should exercise—he's the one who recommended that I try the "mask with transform" (square with the little off-center dot) and "dynamic transform" (sliding magic) cases.