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

[Turbo] Add stream format with request listener #2550

Merged
merged 1 commit into from
Feb 7, 2025

Conversation

aleho
Copy link
Contributor

@aleho aleho commented Feb 5, 2025

Q A
Bug fix? yes
New feature? no
Issues Fix #2549
License MIT

BundleInterface::boot() is only called when booting a worker and not on every request. So on FrankenPHP for example Turbo requests would never return a stream when checking with if ($request->getPreferredFormat() === TurboBundle::STREAM_FORMAT.

This replaces TurboBundle::boot() with a request listener to register the format on every request.

@carsonbot carsonbot added Bug Bug Fix Turbo Status: Needs Review Needs to be reviewed labels Feb 5, 2025
@aleho aleho force-pushed the fix/turbo-stream-format-workers branch from a770555 to 2d28de2 Compare February 5, 2025 10:05
@aleho
Copy link
Contributor Author

aleho commented Feb 5, 2025

The failing tests are in LiveComponents and a timeout in webdriver. I hope they're not related to my changes ;).

@smnandre
Copy link
Member

smnandre commented Feb 6, 2025

Hi @aleho ! Thank you for this PR :)

Do you think you could add a small test to prevent any future regression?

@smnandre smnandre added Status: Reviewing Review is ongoing, refining with author and removed Status: Needs Review Needs to be reviewed labels Feb 6, 2025
@aleho aleho force-pushed the fix/turbo-stream-format-workers branch from 2d28de2 to ae70cf2 Compare February 6, 2025 08:40

Verified

This commit was signed with the committer’s verified signature.
aleho Alexander Hofbauer
BundleInterface::boot() is only called when booting a worker and not on
every request.

Fixes symfony#2549
@aleho aleho force-pushed the fix/turbo-stream-format-workers branch from ae70cf2 to 91b8268 Compare February 6, 2025 08:47
@aleho
Copy link
Contributor Author

aleho commented Feb 6, 2025

@smnandre I wasn't entirely sure how to test that. Is "simulating" a worker mode by not re-booting the kernel enough?

@smnandre
Copy link
Member

smnandre commented Feb 6, 2025

@aleho i think so! cc @kbond ?

Copy link
Member

@kbond kbond left a comment

Choose a reason for hiding this comment

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

Cool, makes sense!

@carsonbot carsonbot added the Status: Reviewed Has been reviewed by a maintainer label Feb 7, 2025
@smnandre smnandre added Feature New Feature and removed Bug Bug Fix labels Feb 7, 2025
@smnandre
Copy link
Member

smnandre commented Feb 7, 2025

Thanks @aleho.

@smnandre smnandre merged commit 266e1a8 into symfony:2.x Feb 7, 2025
63 checks passed
@aleho aleho deleted the fix/turbo-stream-format-workers branch February 10, 2025 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New Feature Status: Reviewed Has been reviewed by a maintainer Status: Reviewing Review is ongoing, refining with author Turbo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Turbo] Request format is not registered for workers
4 participants