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 unnecessary browser polyfills #91329

Closed
3 of 4 tasks
mshustov opened this issue Feb 13, 2021 · 6 comments
Closed
3 of 4 tasks

Remove unnecessary browser polyfills #91329

mshustov opened this issue Feb 13, 2021 · 6 comments
Labels
impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort Team:Operations Team label for Operations Team

Comments

@mshustov
Copy link
Contributor

mshustov commented Feb 13, 2021

I was testing what performance benefits we can gain by migrating to TS v4.2 WIP is here https://github.com/restrry/kibana/tree/ts-4-2-rc
One of the errors was caused by ResizeObserver type incompatibility in lib.d.ts and resize-observer-polyfill package.
I went to http://caniuse.com to check whether we need this polyfill for our browser support matrix
It seems that we can remove:

Not sure that we can get rid of regenerator-runtime/runtime. According to the comment, it's required for EUI

// disable built-in filtering, which is more performant but strips the import of `regenerator-runtime` required by EUI
nevertheless, the comment might be outdated.

@mshustov mshustov added the Team:Operations Team label for Operations Team label Feb 13, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations (Team:Operations)

jbudz added a commit to jbudz/kibana that referenced this issue Apr 6, 2021
polyfills

AbortController and ChildNode.Remove are both covered by our browser
support matrix, now that IE11 has been removed.

https://caniuse.com/childnode-remove
https://caniuse.com/abortcontroller
https://www.elastic.co/support/matrix#matrix_browsers

Part of elastic#91329
jbudz added a commit to jbudz/kibana that referenced this issue Apr 6, 2021
polyfills

AbortController and ChildNode.Remove are both covered by our browser
support matrix, now that IE11 has been removed.

https://caniuse.com/childnode-remove
https://caniuse.com/abortcontroller
https://www.elastic.co/support/matrix#matrix_browsers

Part of elastic#91329
@mshustov
Copy link
Contributor Author

mshustov commented May 23, 2021

ResizeObserver typings are added to lib.d.ts microsoft/TypeScript#28502 (comment)

@jbudz Is it the only reason to keep ResizeObserver polyfill?

@jbudz
Copy link
Member

jbudz commented May 24, 2021

The other reason is it's used in development for test mocking here. We'll have to replace the mock implementation.

@mshustov
Copy link
Contributor Author

We'll have to replace the mock implementation.

We already have a mock https://github.com/elastic/kibana/blob/master/src/plugins/kibana_utils/public/resize_checker/resize_checker.test.ts#L43

We just need to mock native API instead of polyfill. Don't we?

jest.spyOn(window, 'ResizeObserver').mockImplementation(() => {
  // the same code as in the test file
})

@tylersmalley tylersmalley added 1 and removed 1 labels Oct 11, 2021
@exalate-issue-sync exalate-issue-sync bot added impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort labels Oct 12, 2021
@Dosant
Copy link
Contributor

Dosant commented Dec 17, 2021

Didn't see this issue before creating this specifically for resize-observer-polyfill #121509

I think we can remove the polyfill ? I listed all places it is used and tagged relevant teams

@mshustov
Copy link
Contributor Author

@Dosant it might be easier to remove all the places at once. I'm closing the current issue, let's use the one you created to track the effort

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort Team:Operations Team label for Operations Team
Projects
None yet
Development

No branches or pull requests

5 participants