chore: migrate the root template to .heex #2376
Conversation
|
@joshlarson or @anthonyshull might remember this as a repeat of #2233 but that included some other changes and was reverted in #2239 🥲 |
| <meta http-equiv="X-UA-Compatible" content="IE=edge" /> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1" /> | ||
| {csrf_meta_tag()} | ||
| <% meta_description = |
There was a problem hiding this comment.
I think it's best practice to avoid setting variables in components like this, since it can cause unnecessary full-component (in this case, full-page) re-renders. (Got this rec from the pitfalls section of the Assigns & HEEx guide.)
I admit that I'm not sure what the implications are for how layouts act, with the existence of an @inner_block that does do change-tracking properly - maybe it re-renders the template, but keeps the inner block change-tracked? Maybe it re-renders the entire page?
There was a problem hiding this comment.
I'm skeptical it'd affect layouts the same way, but I've added a commit which avoids doing this.
There was a problem hiding this comment.
I don't think it would have an effect unless you're expecting that variable to change. In the example given in the link, they are summing something. In this case it is just a string being set.
thecristen
force-pushed
the
cbj/root-template-heex
branch
from
c312772 to
c8c66ae
Compare
No ticket, just tech debt—This'll make it easier to include function components on the homepage! ✨
Follow the commits one-by-one to see more legible diffs of what changed between each stage.
Dotcom.BodyTag, I had to make an adjustment because HEEX doesn't like the<%= Dotcom.BodyTag.render(@conn) %> </body>format 😓<meta name="robots" content="noindex, nofollow">tag because the current one was driving me nuts. I added a function to handle whether we should show the tag or not.DotcomWeb.Plugs.CanonicalHostnameadds a 301 redirect for that case.Visually this should be a no-op. Going to deploy it to verify it works as expected with
MIX_ENV=prod.