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

patch on str_contains polyfill may conflict with other plugins #133

Open
xoich opened this issue Jun 7, 2021 · 3 comments
Open

patch on str_contains polyfill may conflict with other plugins #133

xoich opened this issue Jun 7, 2021 · 3 comments

Comments

@xoich
Copy link

xoich commented Jun 7, 2021

Hello, I'm talking about this commit: 78cfc99

polyfill80 is a pretty common dependency. If another plugin uses it and is active at the same time as geoip-detect, the version from the other plugin might be loaded. Then we will still have the same error that the commit is trying to prevent.

@xoich xoich changed the title patch on str_contains polyfill will conflict with other plugins patch on str_contains polyfill may conflict with other plugins Jun 7, 2021
@benjaminpick
Copy link
Member

You're right ... and there is not much I can do about it, this should be fixed upstream. And this doesn't seem likely:
symfony/symfony#37035
But they are open for PR.

At some point, I will probably use PhpScoper to scope all my vendor dependencies ... this is my long-term plan ;-) Oh, wait, in this case I can't scope that function because it is polyfilling global! Hm....

@benjaminpick
Copy link
Member

WP 5.9 now includes the polyfill, I have to investigate further ...

@benjaminpick
Copy link
Member

benjaminpick commented Jan 14, 2022

Here is the polyfill from Wordpress - it also allows NULL values without throwing an error. So I guess I can leave my change above.

https://github.com/WordPress/WordPress/blob/b4de570d3555cce5d386f2f10d1d49d2444f3a77/wp-includes/compat.php#L420-L436

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

No branches or pull requests

2 participants