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

Extend compat metadata for URL and URLSearchParams polyfills #853

Closed

Conversation

leipert
Copy link

@leipert leipert commented Aug 10, 2020

Certain versions of edge and safari support URL, URLSearchParams and
URL.prototype.toJSON. However the compat data doesn't seem to note that,
thus leading for unnecessary polyfilling.

The updated data has been taken from MDN and caniuse:

Certain versions of edge and safari support URL, URLSearchParams and
URL.prototype.toJSON. However the compat data doesn't seem to note that,
thus leading for unnecessary polyfilling.

The updated data has been taken from MDN and caniuse:

- https://developer.mozilla.org/en-US/docs/Web/API/URL#Browser_compatibility
- https://caniuse.com/#feat=url
- https://caniuse.com/#feat=mdn-api_url_tojson
- https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams#Browser_compatibility
- https://caniuse.com/#feat=urlsearchparams
@leipert
Copy link
Author

leipert commented Aug 10, 2020

@slowcheetah Do you mind reviewing this?

},
'web.url-search-params': {
chrome: '67',
firefox: '57',
chrome: '61',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this version of Chrome does not support the full feature set of url search params. I can't remember what is missing but the test suite does cover it

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mhm, according to https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams#Browser_compatibility it should support it. .sort was added in that version

Copy link
Author

@leipert leipert Aug 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However bumping it up to 67 again would be fine for me.

@dilyanpalauzov
Copy link

See also #656 (comment) .

Only in Safari (WebKit 2.28.4) "new URL('https://x', undefined)" throws an error, so polyfill for it is kind of OK.

But I do not see what fails with new URLSearchParams() so that for it also a polyfill is necessary.

@dilyanpalauzov
Copy link

dilyanpalauzov commented Oct 18, 2020

The change above is about "new URL" and "new URLSearchParams".

In Safari new URL(x, undefined) does not work, I have communicated this to

All of the above state, than in Safari 14.1 the problem will disappear.

The defect for new URLSearchParams in the core-js database was introduced at the same time, (so for the same reasons), when the defect for new URL(x, undefined) was detected. However, for new URLSearchParams() itself there was per #656 never a problem.

I send my code to terser and I use const urlbar = new URLSearchParams(window.location.search) to get the URL parameters in JavaScript of the requested webpage. If I remove that line my uncompressed code is reduced by 28kb. If I compress the code with brotli and compare the difference of the size, the difference is 9.7kb.

The motivation for the change can be expressed in different ways. One of the ways is:

Skipping not needed polifills in core-js for "URLSearchParams" leads to smaller JavaScipt files, that user download from internet. Smaller files means less electricity is necessary for the transmission and less electricity implies less pollution. By releasing a new version of core-js, which does not send polifills for new URLSearchParams for any version of Safari (excepth there is evidence for the opposite) and does not send for Safari 14.1 or later polifills for new URL(), core-js contributes to reducing the global pollution.

If there are difficulties to tackle the change as a whole, please split it in smaller pieces and make progress with whatever is clear.

@zloirock
Copy link
Owner

core-js compat data is the result of calling the core-js test case (for URL and URLSearchParams it's this code) in different engines which is an equal of core-js feature detection in the library code. Moreover, it's a very relaxed test that does not cover many engines bugs. Makes no sense to rely on data from MDN or caniuse which is not related to this test case and just to change this data without changing those tests.

Add to the compat data new results of those tests? Maybe, because all this year I was in prison and for this time were changed some generations of browsers. But... I don't see anything that could be added. Modern versions of Edge get data from the chrome field. What about Safari? iOS 14.1 shows that those tests are not passed. My MacBook is dead, Safari 14 still not available in Saucelabs, so I can't check the details.

I send my code to terser and I use const urlbar = new URLSearchParams(window.location.search) to get the URL parameters in JavaScript of the requested webpage. If I remove that line my uncompressed code is reduced by 28kb. If I compress the code with brotli and compare the difference of the size, the difference is 9.7kb.

You use it in this way, someone in another - and bugs from the feature detection could affect them if they will not get this polyfill. Sure, the detection of used features in injection plugins could be smarter, I have some ideas about it - but anyway it's very hard work and definitely will be improved not soon. If you are completely sure that this module is not required in your case - you could add it to the blacklist of the plugin - and it will not be injected.

Skipping not needed polifills in core-js for "URLSearchParams" leads to smaller JavaScipt files, that user download from internet. Smaller files means less electricity is necessary for the transmission and less electricity implies less pollution. By releasing a new version of core-js, which does not send polifills for new URLSearchParams for any version of Safari (excepth there is evidence for the opposite) and does not send for Safari 14.1 or later polifills for new URL(), core-js contributes to reducing the global pollution.

Because of the big words like this, I have no ideas about how to charge my iPhone.

@zloirock zloirock closed this Oct 27, 2020
@Hypnosphi
Copy link

Can the tests for URL and URLSearchParams be separated from each other?

@zloirock
Copy link
Owner

Nope. They are too tied under the hood by the spec.

PaperStrike added a commit to PaperStrike/Pjax that referenced this pull request Sep 25, 2021
* Submission lib uses URLSearchParams and at some point Babel started to polyfill it. The compat data core-js-compat can't give the root cause for the Safari 14 requirement[^1]. Since other compat data like MDN shows it only needs Safari 11[^2], exclude it from the bundle.

[^1]: zloirock/core-js#853 (comment)
[^2]: https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams#browser_compatibility
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.

None yet

5 participants