-
-
Notifications
You must be signed in to change notification settings - Fork 83
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
Reset assets on FINISH_REQUEST. #115
Conversation
bffe88a
to
dbc29fa
Compare
dbc29fa
to
029b01b
Compare
Sorry for the slow merge - thank you @Warxcell! |
@weaverryan After upgrading to v1.14.0 I have following error: What can I do to have the runtime.js only on top? |
Hm, do you have sub-requests in-between? Can you provide reproducible code in some repo so I can fork and add a fix? Or directly add test case ? |
@Warxcell Yes there are sub-requests in-between. I can try to give you an reproducible code example. |
We're seeing similar issues with duplicate runtime.js script tags for our code. I see use cases both ways, sometimes you want your all script tags on every request, sometimes you don't. To me, that makes it feel like the option should be configurable somewhere. |
@Warxcell @weaverryan We also ran into the problem, that one of our projects kept loading the runtime.js multiple times. In our case it lead to forms being sent multiple times (including submission emails). Really had to look deep to figure out why it did that. As already stated; from a developers perspective I totally understand why you implemented that, but I think this sub-request thingy could break a lot of stuff. We were happy that it was "just" a contact form. I was wondering what would've happened if we updated the bundle in one of our larger shop systems (multiple orders? multiple items in the cart by accident? etc etc). Unfortunately I can't really think of a "better" alternative than the Request-Finished event. But I think it really makes sense to think about it, how it could impact users. Thank you, stay healthy! BR, |
@Warxcell as @zing-james explained in #166, if you use the twig render function you end up with runtime.js being load twice, this is a huge side effect as I believe many people (including myself) use this in their projects. |
Hi @Warxcell, @weaverryan, I am having the same issue with embed controller after the update. I have check the other use cases where users want to reset the assets. I want to provide a suggestion that if we can differentiate the sub requests then we can put the condition when we want to reset and when we don't want to. 1 option is to add attribute in request similar to _format eg. _renderer : inline / fragment waiting for feedback. Happy to create PR for Symfony and add attribute and then create PR to fix the issue. |
Sounds ok to me. |
Fixes #90, #94