slime-lang / phoenix_slime

Phoenix Template Engine for Slime
MIT License
310 stars 66 forks source link

Revise Slime's sigil use in light of LiveView #80

Closed mtrudel closed 3 years ago

mtrudel commented 4 years ago

With LiveView coming on the scene, Slime's use of sigils could stand a review. Specifically, I see two issues that may warrant attention:

  1. Slime's use of ~l and ~L as inline content sigils conflict with LiveView sigils of the same name. Being as LiveView is likely sticking around, it stands to reason that this conflict should be resolved before there's too much precedent. Note that while one can sidestep this by selectively importing LiveView (import Phoenix.LiveView.Helpers, except: [sigil_L: 2] is what I'm using), it's one more step to integrate, and as such it may be worth tearing the band-aid off early on this.

  2. Slime lacks a set of sigils for inline LiveView, which would be quite handy as LiveView patterns tend to prefer inline markup. (Note that the actual implementation of such a sigil looks pretty straightforward based on reviewing how the LiveView project does it).

To stimulate discussion on this, I'd like to propose three options:

  1. Double down on Slime as a replacement for EEx and start using ~e / ~E for non-live slime, and ~l / ~L for live-slime. This has the downside of not coexisting at all with EEx's sigils, but also the upside of being pretty explicit (and clear to switch between based on selective imports of one or the other as above).

  2. Do our best to accommodate LiveView (on the premise that it's only going to get more popular over time) and update Slime's sigils to something other than ~l / ~L. I might suggest using ~i and ~I for non-live slime, and ~m and ~M for live slime (since those are the next characters in the word 'slime'), but that's just an idea.

  3. Keep the status quo and update documentation as appropriate. Either pick another character for a live-slime sigil, or use a trailing modifier on the existing sigil to indicate live-ness (side note: it's annoying that LiveView didn't do this, though understandable given that it's a distinction that spans two libraries).

I'm quite happy to work up a PR to implement any of the above ideas, but I'd like to solicit community feedback on them before proceeding (FWIW, I think I prefer option 1 but I'll admit to not having thought it through super deeply).

mtrudel commented 4 years ago

Relevant to #61, #63, #72

johns10 commented 3 years ago

Would there be some other way of differentiating between which sigil is being used? For example, if I wanted to migrate my live_view app to slime, how could I use regular ~L in my old templates, and ~sLime in my other templates?

johns10 commented 3 years ago

Here's how I made this work on my project. Note that I'm using the standard generated phoenix project. It comes with some baked in definitions in the my_web_app.ex file that looks like this:

  def live_component do
    quote do
      use Phoenix.LiveComponent
      unquote(view_helpers())
    end
  end

view_helpers looks like:

  defp view_helpers do
    quote do
      use Phoenix.HTML
      import UserDocsWeb.LiveHelpers
      import Phoenix.LiveView.Helpers
      import Phoenix.View
      import UserDocsWeb.ErrorHelpers
      import UserDocsWeb.Gettext
      alias UserDocsWeb.Router.Helpers, as: Routes
    end
  end

I moved the import Phoenix.LiveView.Helpers into the live_component call, then added this:

  def live_slime_component do
    quote do
      use Phoenix.LiveComponent
      import Phoenix.LiveView.Helpers, except: [sigil_L: 2]
      import PhoenixSlime, only: [ sigil_L: 2 ]
      unquote(view_helpers())
    end
  end

Now when I want to use live_slime in one of my components, I use:

use UserDocsWeb, :live_slime_component

instead of

use UserDocsWeb, :live_component

Nothing earth shattering here but it works.

johns10 commented 3 years ago

This actually only worked in the case where I was calling ModuleName.render, and doesn't work on render callbacks in components.

doomspork commented 3 years ago

I'm working on updating Slime and this package this week. Are there any suggestions for a better sigil?

mtrudel commented 3 years ago

Having thought about this, I don't think any of the options are great. I think perhaps option 1 w/ some prominent notices in the docs & readme may be the best, but I don't like any of the options at hand, TBH.