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

Add links to some core-js polyfills #4838

Merged
merged 1 commit into from
May 26, 2021
Merged

Add links to some core-js polyfills #4838

merged 1 commit into from
May 26, 2021

Conversation

zloirock
Copy link
Contributor

@zloirock zloirock commented May 9, 2021

No description provided.

@zloirock zloirock requested review from a team as code owners May 9, 2021 21:05
@zloirock zloirock requested review from Rumyra and removed request for a team May 9, 2021 21:05
@Elchi3
Copy link
Member

Elchi3 commented May 10, 2021

Hey @zloirock, thanks for your PR!
Did you see we started to collect thoughts on how MDN should deal with Polyfills? openwebdocs/project#27 What do you think about it?

@zloirock
Copy link
Contributor Author

zloirock commented May 10, 2021

@Elchi3 I didn't see this thread, but I saw some related. I agree that MDN should not contain polyfills directly.

I already had problems because of incorrect / incomplete MDN polyfills some times, for example, zloirock/core-js#702. In addition, I see some links to incorrect / incomplete polyfills on MDN pages. To fix it at least somehow, I made this PR.

core-js is the most popular polyfill of JS standard library, used on most websites (even a naive detection shows ~59% on TOP 1000 websites) and contains proper polyfills.

I didn't remove polyfills code from MDN since I'm not sure that it's in the scope of this PR.

@github-actions

This comment has been minimized.

@sideshowbarker
Copy link
Collaborator

I’d be happy for this to get merged. Having polyfill links in the See Also section at the end of articles is about as unobtrusive as it could be made — I mean as far as the goal of moving away from having polyfill code within the articles themselves.

We’ve also in this issue tracker had feedback from developers saying they quite like being able to get to polyfill info from MDN articles — so this PR seems to do a pretty great job of serving that need.

Also, I think — given the visibility that core-js has — the core-js polyfills have been vetted much more carefully and thoroughly than anything else out there may have been (and continue to get vetted and fixed and needed) and so we can have high confidence that linking from MDN to them is pointing developers at a quality resource.

As far as the question of removing the polyfill code we still have within the articles themselves, it seems not necessary to block this PR on that being done — the removals can be done in a subsequent PR. (I think in fact that given the comprehensiveness/thoroughness of this PR, we could just now confidently remove all remaining polyfill code from any articles in the javascript tree which have them — without worrying that it’d result in any information loss for developers.)

@Rumyra
Copy link
Collaborator

Rumyra commented May 26, 2021

I agree with @sideshowbarker - I think these resources add value. If no one disagrees I will go ahead and merge this 👍

Copy link
Member

@Elchi3 Elchi3 left a comment

Choose a reason for hiding this comment

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

Yes, I hope to continue the conversation on Polyfills but please merge this, @Rumyra 👍

@Rumyra Rumyra merged commit 3c394e6 into mdn:main May 26, 2021
@sideshowbarker
Copy link
Collaborator

Denis, thanks much, and congrats on landing your first docs change here — welcome aboard 🎉

@zloirock zloirock mentioned this pull request Sep 22, 2021
@zloirock zloirock mentioned this pull request Dec 24, 2021
@zloirock zloirock mentioned this pull request Feb 2, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants