search

justbetter / magento2-sentry

Magento 2 module to log to Sentry
MIT License
165 stars 70 forks source link

remove promise-polyfill #142

Closed rommelfreddy closed 3 months ago

rommelfreddy commented 4 months ago

This PR removes the Polyfill for Promises.

Magento does not Support IE anymore in the latest releases, so we do not need the Promise-Polyfill, because all browsers does support Promises (fully)

Please also see: https://caniuse.com/?search=promise (I think we can ignore the Opera Mini)

rommelfreddy commented 4 months ago

as an alternative: it should be included by the extension directly an not via an external ressource to prevent issues like this:

indykoning commented 4 months ago

I agree with this, it's no longer needed. I'll keep this PR open for (maybe) a few releases more and then completely remove our dependency on it 😁

rommelfreddy commented 4 months ago

I think you can safely merge it: image The file is completly empty.

indykoning commented 3 months ago

Yes, on most browsers it will be. However try to access the js file via curl

curl "https://polyfill-fastly.io/v3/polyfill.min.js?features=Promise"

and you can see it does return content. It dynamically returns a polyfill depending on if it thinks the browser needs it

rommelfreddy commented 3 months ago

indeed. but i still think that it is not required anymore to keep this polyfill, because we have a global support of more than 97%. I also mean that Sentry itself should be responsible for the Promise-Polyfill and not the Magento module.

And be honest: what should happen, if the promise is missing -> just an error. The website keeps working. And if the Promise is missing, i think there is much more missing within the browser, so the latest Magento versions would also no work within this browser.

Let's drop the support of Magento 2.0 - 2.3 and then remove the polyfil. IE is not supported by Magento 2.4 anymore, and the Promise is missing in IE (with the largest usage and missing Promise).

And then upgrade the code to PHP 8 (with rector and ecs) and the module look very pretty ;-)

indykoning commented 3 months ago

Agreed, anyone using IE has serious security issues and will probably fill up Sentry with only IE related errors