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

Wrapping up t-prefix fix #89

Merged
merged 2 commits into from
Aug 31, 2023
Merged

Wrapping up t-prefix fix #89

merged 2 commits into from
Aug 31, 2023

Conversation

kaspth
Copy link
Contributor

@kaspth kaspth commented Jun 27, 2023

Follow-up to #86 using a different branch since a client project is relying on that branch now.

Closes #86.

@kaspth
Copy link
Contributor Author

kaspth commented Jun 27, 2023

Once bullet-train-co/bullet_train-core#331 has been merged, we can remove d43289a and merge this to ship a new Nice Partials release.

@kaspth kaspth marked this pull request as ready for review June 27, 2023 12:31
@kaspth kaspth marked this pull request as draft June 27, 2023 16:27
@kaspth kaspth marked this pull request as ready for review August 30, 2023 17:50
@kaspth
Copy link
Contributor Author

kaspth commented Aug 30, 2023

I rebased out the in-flight Super Scaffolding testing, so now I'll wait for the build and then we can go.

This fixes a change I made to our t-prefixes that broke things: overriding `ActionView::Base#capture` meant that we'd fire on helper calls too — and extract from their block location.

Note: we're leveraging Rails 6.1+'s `@current_template.virtual_path` to avoid needing to muck with block source_locations or view_paths,
Rails has already figured that out for us.
`t` might be called often on a page and in a nested loop these needless String allocations could add up.
Since they're so easy to fix, let's do it.
@kaspth kaspth merged commit cd6e80e into main Aug 31, 2023
3 checks passed
@kaspth kaspth deleted the fix-t-prefix-part-2 branch August 31, 2023 16:00
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.

None yet

1 participant