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.

Sign up for GitHub

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

Jump to bottom

Replace JS toolchain by asset-mapper when testing ux-turbo #2232

Merged
merged 1 commit into from
Oct 3, 2024
Merged

Replace JS toolchain by asset-mapper when testing ux-turbo #2232

merged 1 commit into from
Oct 3, 2024

Conversation

nicolas-grekas
Copy link
Member

Q A
Bug fix? no
New feature? no
Issues -
License MIT

🙊

@carsonbot carsonbot added the Status: Needs Review Needs to be reviewed label Oct 2, 2024
@nicolas-grekas nicolas-grekas force-pushed the asset-mapper-ftw branch 4 times, most recently from 1907c84 to c7090d4 Compare October 2, 2024 18:15
@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented Oct 2, 2024 •
edited
Loading

Curiously, this fails on PHP 8.3 but passes on 8.4 (deps=high).
It's going to be fun to debug... If anyone is up to having a look, help welcome :)

@nicolas-grekas nicolas-grekas force-pushed the asset-mapper-ftw branch 2 times, most recently from 38f6c29 to b0f7da9 Compare October 2, 2024 19:07
@Kocal
Copy link
Member

Kocal commented Oct 2, 2024

I will take a look tomorrow!

@Kocal Kocal force-pushed the asset-mapper-ftw branch 5 times, most recently from da14a70 to 2264ff2 Compare October 3, 2024 06:59
@Kocal
Copy link
Member

Kocal commented Oct 3, 2024

Those issues were caused because the assets were not reachable with the webserver/browser ran by Panther:
image

It was fixed by compiling assets with asset-map:compile.

Now all that's left is this issue and we'll be good to go :)
image

@nicolas-grekas
Copy link
Member Author

It was fixed by compiling assets with asset-map:compile.

Super strange. I opened php/php-src#16194 because this looks like a PHP bug to me.

@nicolas-grekas
Copy link
Member Author

Thanks for the help @Kocal

PR is ready.

@carsonbot carsonbot added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Review Needs to be reviewed labels Oct 3, 2024
@javiereguiluz
Copy link
Member

Nice one! Thanks Nicolas

@javiereguiluz javiereguiluz merged commit c23d756 into symfony:2.x Oct 3, 2024
63 checks passed
@Kocal Kocal mentioned this pull request Oct 12, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kocal Kocal Kocal approved these changes

Assignees
No one assigned
Labels
Status: Reviewed Has been reviewed by a maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nicolas-grekas @Kocal @javiereguiluz @carsonbot