geoip-detect icon indicating copy to clipboard operation
geoip-detect copied to clipboard

Published 20 hours ago • verified publisher icon yellowtree

patch on str_contains polyfill may conflict with other plugins

Open xoich opened this issue 4 years ago • 3 comments

Hello, I'm talking about this commit: https://github.com/yellowtree/geoip-detect/commit/78cfc99a0087ad46cbc61699fd41563b2fb74763

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 avatar Jun 07 '21 12:06 xoich

You're right ... and there is not much I can do about it, this should be fixed upstream. And this doesn't seem likely: https://github.com/symfony/symfony/issues/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 avatar Jun 18 '21 08:06 benjaminpick

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

benjaminpick avatar Jan 13 '22 18:01 benjaminpick

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

benjaminpick avatar Jan 14 '22 08:01 benjaminpick