Skip to content

Fix caching #279

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

Merged
merged 3 commits into from
Mar 17, 2025
Merged

Fix caching #279

merged 3 commits into from
Mar 17, 2025

Conversation

joeldrapper
Copy link
Collaborator

Comment on lines 10 to 18
original_capturing = @capturing
original_fragments = @fragments

begin
@capturing = true
yield
ensure
@capturing = original_capturing
@fragments = original_fragments
capture_context.capture do
yield
ensure
@capturing = original_capturing
@fragments = original_fragments
end
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure I understand what's going on with the @capturing and @fragments here. Are they expected to be mutated inside the yield? Why are we saving their state and restoring it, but not (seemingly) making any changes to them?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ops, I missed something. Let me fix this.

joeldrapper added a commit to yippee-fun/phlex that referenced this pull request Mar 17, 2025
In order to fix yippee-fun/phlex-rails#276, we
need to simplify the capturing interface so it can be swapped out. This
PR removes the ability to capture into an arbitrary buffer. Instead, it
depends on reading the new buffer from the state within a capture block.

See yippee-fun/phlex-rails#279
Copy link
Collaborator

@willcosgrove willcosgrove left a comment

Choose a reason for hiding this comment

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

This looks good to me!

@joeldrapper joeldrapper marked this pull request as ready for review March 17, 2025 18:26
@joeldrapper joeldrapper merged commit 57da235 into main Mar 17, 2025
9 checks passed
@joeldrapper joeldrapper deleted the fix-caching branch March 17, 2025 18:26
@peterberkenbosch
Copy link

🎉 🎉 🎉

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