Skip to content

HEEx :extract protocol#3725

Closed
SteffenDE wants to merge 4 commits intosd-colo-hooksfrom
sd-colo-hooks-protocol
Closed

HEEx :extract protocol#3725
SteffenDE wants to merge 4 commits intosd-colo-hooksfrom
sd-colo-hooks-protocol

Conversation

@SteffenDE
Copy link
Collaborator

More generic implementation for #3705.

TODO

  • Tests
  • Docs

@SteffenDE SteffenDE force-pushed the sd-colo-hooks-protocol branch from dc2b345 to 27b498e Compare March 24, 2025 17:58
@SteffenDE SteffenDE requested a review from josevalim March 25, 2025 16:07
Comment on lines 197 to 215
def postprocess_tokens(
%Phoenix.LiveView.ColocatedHook{name: hook_name},
%{hashed_name: hashed_name},
tokens
) do
Enum.map(tokens, fn
{:tag, name, attrs, meta} ->
{:tag, name, rewrite_hook_attrs(hook_name, hashed_name, attrs), meta}

{:local_component, name, attrs, meta} ->
{:local_component, name, rewrite_hook_attrs(hook_name, hashed_name, attrs), meta}

{:remote_component, name, attrs, meta} ->
{:remote_component, name, rewrite_hook_attrs(hook_name, hashed_name, attrs), meta}

other ->
other
end)
end
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One idea I had to not have to make the tokens public is to basically treat them as opaque and have a module with functions for working with them, e.g.

  def postprocess_tokens(
        %Phoenix.LiveView.ColocatedHook{name: hook_name},
        %{hashed_name: hashed_name},
        tokens
      ) do
    alias Phoenix.LiveView.TagExtractorHelper

    TagExtractorHelper.map_tokens(tokens, fn token ->
      TagExtractorHelper.rewrite_attribute(token, "phx-hook", fn _old_value -> hashed_name end)
    end)
  end

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went ahead with that approach.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SteffenDE SteffenDE changed the base branch from main to sd-colo-hooks March 25, 2025 16:14
@SteffenDE
Copy link
Collaborator Author

I changed the base branch to the other colocated hook branch for easier review.

@greven
Copy link

greven commented Mar 26, 2025

First and foremost, amazing work @SteffenDE! The CSS parser is amazing. 🥲
In your example project (still digesting, there's a lot there), right now the CSS parser is in user land, I guess you are just testing the protocol implementation, but is the plan to extract it to a Phoenix Package?

Exciting stuff, still checking out the implementation and will leave any comments if noteworthy.

@SteffenDE
Copy link
Collaborator Author

@greven the CSS parser/generator is not my work, but taken for demo purposes from a project linked on the elixir forum:
https://elixirforum.com/t/to-have-mix-phx-new-no-tailwind-to-just-do-not-use-tailwind-but-still-create-basic-css/69906/15
https://git.sr.ht/~garrisonc/corp_style

It doesn’t have a license, so I may have to take it down on request.

The plan is not to have such code in LiveView, but instead let other people implement it as libraries, while LV provides the building blocks.

Comment on lines +36 to +40
defp remove_outdated_extracts(manifest_extracts) do
extracts = get_extracts(project_modules())

case manifest_extracts -- extracts do
[] ->
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's one issue here in that if you include the extracted files in your git repo and then remove modules without ever compiling them, the files won't be removed

@SteffenDE SteffenDE mentioned this pull request Mar 28, 2025
@JonRowe
Copy link
Contributor

JonRowe commented Apr 29, 2025

As someone who is interested in co-located css this is a very interesting PR 😂

@SteffenDE
Copy link
Collaborator Author

We're going to rework this.

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

Successfully merging this pull request may close these issues.

3 participants