-
Notifications
You must be signed in to change notification settings - Fork 486
[v4] Troubleshooting issue with Slim #2409
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
base: main
Are you sure you want to change the base?
Conversation
|
note: today i found other two apps where v4 + Slim caused the CI to fail with the same errors |
|
@edolix thank you for the report and for following up about seeing this issue elsewhere. I too was unable to reproduce it in https://github.com/ViewComponent/demo. (I've now added Slim to the test suite so you can try to reproduce it there if you'd like) Please keep us posted as you work to reproduce it in isolation- we don't use Slim at GitHub so I can't dedicate much time to this issue at the moment ❤️ |
|
@joelhawksley i found a way to reproduce it in this repo: https://github.com/edolix/view-component-bug-replica I added piece by piece until i got the same behavior of my apps - i'm adding more info in the README but let me know if you see the same errors 🙏 |
|
@joelhawksley sorry to bug you directly but just to confirm: can you try to run the repo to validate the issue with Slim please? Thank you so much! 🙏 |
|
@edolix sorry, I haven't had the time to get to this. As GitHub does not use Slim, I'm not able to prioritize this PR over a couple of work priorities I have on my plate. If you'd like to work on a fix, I'd happily review what you come up with ❤️ |
|
@joelhawksley no worries at all! I 💯 understand! As soon as I have some free time I'll try again 🤞 |
|
@edolix thanks again for providing the replica repo. I used it to narrow down the failure to 4.0.0.alpha2...4.0.0.alpha3, basically when we removed the dependency on ActionView::Base. cc @camertron do you have any ideas? |
|
This should be fixed by #2547 😄 |
What are you trying to accomplish?
Trying to upgrade ViewComponent to v4 in one of our apps, @xkraty and i noticed a lot of failures in CI like:
Couldn't find a template file or inline render method for SomeSlimComponent.... I was like 🤔We use Slim everywhere and everything worked as expected with v3.
I tried to dig a bit and i think it could be a race condition between Slim initialization and the cache logic for each component.
I may be wrong so apologies in case all of this doesn't make sense!
In order:
In
ViewComponent::Base inheritedeach component will be automatically compiled (and then cached) as soon as the file is loaded:view_component/lib/view_component/base.rb
Lines 544 to 547 in 6a33362
The problem comes with the Slim gem that will register the
slimtemplate handler only at theaction_viewhook causing a (possible) race condition within theViewComponent::Compiler.gather_templatesmethod:view_component/lib/view_component/compiler.rb
Lines 177 to 179 in 6a33362
ActionView::Template.template_handler_extensionsmight not include "slim" at that point.All templates are gonna be memoized (with the related
template_errorsas well).If few components got loaded (and automatically cached due to
ViewComponent::Base.inherited) before theaction_viewhook (that will register slim as a valid template handler)... those components will always raise theCouldn't find a template file or inline render method for ....What approach did you choose and why?
We created a Rails app trying to reproduce the issue with no luck. I'll try again tomorrow on it.
This PR is a basic/raw/dumb attempt to - at least - highlight the issue and give more context.
Anything you want to highlight for special attention from reviewers?
Nothing - just thanks for the great VC gem! 🙌