8000 More precisely initializing services/config for asset mapper by weaverryan · Pull Request #911 · symfony/ux · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

More precisely initializing services/config for asset mapper #911

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

Merged

Conversation

weaverryan
Copy link
Member
Q A
Bug fix? yes
New feature? no
Tickets Fix #907
License MIT

Hi!

A few fixes for the new 2.9 version, which included a lot of under-the-hood changes to support asset mapper. These shouldn't be noticed by users and hopefully this will smooth out the final bumps:

A) Avoid prepending the asset_mapper config unless FWBundle 6.3 is present. I don't love the solution - but it should work well enough, especially as this applies to asset mapper users and that is still experimental.

B) Avoid registering a few asset mapper services that, on older versions of PHP/Symfony, may cause a container explosion due to a trait that won't be present.

C) Plug some holes in the CI to test lower deps. The CI really needs to become much more dynamic than it is now, but this makes sure that all packages are tested for highest/lowest deps.

Cheers!

return false;
}

return is_file($bundlesMetadata['FrameworkBundle']['path'].'/Resources/config/asset_mapper.php');
Copy link
Member Author

Choose a reason for hiding this comment

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

@stof I don't love this solution, but I can't think of a better option. I'm wondering if, in asset-mapper 6.4, we should allow a service-based way of adding "paths" instead of relying on prepending.

Copy link
Member

Choose a reason for hiding this comment

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

I don't have a better idea for that for now

@weaverryan
Copy link
Member Author

Low deps on 8.1 are failing but should be good (I'll verify after the merge). The CI matrix doesn't patch (yet) cross-dependencies as well as symfony/symfony, so those bundles are not using StimulusBundle from this PR and are thus failing due to its prepend configuration acting when it shouldn't.

@weaverryan weaverryan force-pushed the more-precisely-init-for-asset-mapper branch from 2c218f9 to 34e7ec1 Compare May 30, 2023 14:40
@weaverryan weaverryan merged commit b31ef35 into symfony:2.x May 30, 2023
@weaverryan weaverryan deleted the more-precisely-init-for-asset-mapper branch May 30, 2023 14:41
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.

Can't upgrade 2.8.1 packages to 2.9.0
2 participants
0