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

Fix refresh of dynamic templates #1786

Merged
merged 2 commits into from
Jan 21, 2025
Merged

Fix refresh of dynamic templates #1786

merged 2 commits into from
Jan 21, 2025

Conversation

lawik
Copy link
Contributor

@lawik lawik commented Jan 19, 2025

No description provided.

@lawik lawik requested a review from joshk January 19, 2025 11:13
@joshk
Copy link
Collaborator

joshk commented Jan 19, 2025

Ahhhh I see how it works now!

Doesn't this mean that changes to the non new template will no longer trigger a reload correctly?

@lawik
Copy link
Contributor Author

lawik commented Jan 19, 2025

Good catch. Probably. I had this at compile-time if you recall.

It may be possible to duplicate the module attributes and get both...

@lawik
Copy link
Contributor Author

lawik commented Jan 19, 2025

I am having a hard time reproducing that it would not refresh :D

@joshk
Copy link
Collaborator

joshk commented Jan 19, 2025

I've give this a whirl tomorrow

@lawik
Copy link
Contributor Author

lawik commented Jan 20, 2025

I tried adding both and that seemed to work. But then I couldn't get it to not refresh when I removed them.

May need a mix compile --force to change the behavior but not quite sure.

@joshk joshk self-assigned this Jan 20, 2025
@joshk
Copy link
Collaborator

joshk commented Jan 21, 2025

I found https://hexdocs.pm/mix/1.14/Mix.Tasks.Compile.Elixir.html#module-__mix_recompile__-0 (from https://elixirforum.com/t/external-resource-for-folder-how-does-embed-templates-do-it/55823), which shows we can define @external_resource twice!

I tested it locally and it fixes it for me!

@joshk
Copy link
Collaborator

joshk commented Jan 21, 2025

Not to say your fix didn't work, although granted I haven't checked it yet.

@lawik
Copy link
Contributor Author

lawik commented Jan 21, 2025

Double it!

@joshk joshk enabled auto-merge (squash) January 21, 2025 22:44
@joshk joshk merged commit a086447 into main Jan 21, 2025
2 checks passed
@joshk joshk deleted the dynamic-template-fix branch January 21, 2025 22:49
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.

2 participants