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

Unneeded Symfony/Yaml dependency? (and bundled dev dependencies) #247

Closed
pjeby opened this issue Sep 5, 2019 · 9 comments · Fixed by #374
Closed

Unneeded Symfony/Yaml dependency? (and bundled dev dependencies) #247

pjeby opened this issue Sep 5, 2019 · 9 comments · Fixed by #374

Comments

@pjeby
Copy link
Contributor

pjeby commented Sep 5, 2019

I've been running into some problems with a wp-cli command that needs a newer version of Symfony/Yaml than is vendored with this plugin, and this plugin ends up blocking it from being loaded because of autoloader precedence.

However, I can't seem to find where the Yaml component is actually used by anything within the Cloudflare plugin, so the dependency appears redundant. (In addition, the bundled vendor/ directory appears to have been built with development dependencies, meaning the distribution is unnecessarily large and prone to overriding other packages with outdated versions as well.)

Perhaps symfony/yaml can now be removed from composer.json and the vendor tree rebuilt with just the non-development dependencies? Thanks.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the Stale label Jun 27, 2020
@pjeby
Copy link
Contributor Author

pjeby commented Jun 27, 2020

Further activity such as?

@github-actions github-actions bot removed the Stale label Jun 28, 2020
@jenkoian
Copy link

jenkoian commented Oct 14, 2020

I think it's only used in a single test (

class ParserTest extends TestCase
) ...please consider at the very least to move into a dev dependency. Order of preference:

This relates to #304 a little but this yaml dep is causing us a few problems which I think can be easily resolved. Thanks!

@jenkoian
Copy link

Every time I want to clear the object cache I have to disable this plugin due to this issue...obviously it's not all about me, but any movement on this seemingly small change would be greatly appreciated 🙏 .

If you specify which is your favoured approach I'm more than happy to provide a PR 😇

@jenkoian
Copy link

jenkoian commented Dec 2, 2020

I actually don't think this will help because the autoloader won't change which is where the problem lies, so can rule that one out

Another option though is to use https://github.com/coenjacobs/mozart this is used by some major plugins already,

@jacobbednarz
Copy link
Member

if i understand the issue correctly here, i think this has been fixed since 4.1.0. feel free to let me know if it isn't addressed for you and we can revisit.

@pjeby
Copy link
Contributor Author

pjeby commented Feb 16, 2021

While 4.10 removed the other bundled dev dependencies, symfony/yaml is still listed in require rather than require-dev, still vendored by the plugin, and still autoloaded by it. So the original problem still exists.

@jacobbednarz
Copy link
Member

thanks for confirming! do you want to take a pass at addressing this?

@pjeby
Copy link
Contributor Author

pjeby commented Feb 16, 2021

I took a shot at it in #374.

@jacobbednarz jacobbednarz linked a pull request Feb 16, 2021 that will close this issue
jacobbednarz added a commit that referenced this issue Feb 16, 2021
Fix #247 - unneeded symfony/yaml dependency
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 a pull request may close this issue.

3 participants