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

remove promise-polyfill #142

Merged
merged 2 commits into from
Aug 9, 2024

Conversation

rommelfreddy
Copy link
Contributor

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
Copy link
Contributor Author

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

@indykoning
Copy link
Member

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
Copy link
Contributor Author

rommelfreddy commented Jul 4, 2024

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

@indykoning
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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

@indykoning indykoning merged commit 1a87259 into justbetter:master Aug 9, 2024
2 checks passed
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.

2 participants