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 usage of non-standardised Reflect.decorate. #36408

Closed
wants to merge 1 commit into from
Closed

Remove usage of non-standardised Reflect.decorate. #36408

wants to merge 1 commit into from

Conversation

JakeChampion
Copy link

Reflect.decorate is not on a standards track and has no active proposal being championed in TC39.
We have no idea if Reflect.decorate will get added to the language, even if it does, we have no guarantee it will be the implementation we are assuming it to be.

By having this condition in the code, we are likely preventing Reflect.decorate from ever existing in the language, just like Array.prototype.flatten had to get a different name because MooTools would attempt to use it if it existed and it was not the implementation MooTools assumed it would be.

Here is a W3C TAG finding which recommends against usage of speculative features -- https://www.w3.org/2001/tag/doc/polyfills/#don-t-use-speculative-polyfills-that-defer-to-native-implementations

Please verify that:

  • There is an associated issue in the Backlog milestone (required)
  • Code is up-to-date with the master branch
  • You've successfully run gulp runtests locally
  • There are new or updated unit tests validating the change

Fixes #

`Reflect.decorate` is not on a standards track and has no active proposal being championed in TC39.

We have no idea if `Reflect.decorate` will get added to the language, even if it does, we have no guarantee it will be the implementation we are assuming it to be.

By having this condition in the code, we are likely preventing `Reflect.decorate` from ever existing in the language, just like `Array.prototype.flatten` had to get a different name because MooTools would attempt to use it if it existed and it was not the implementation MooTools assumed it would be.
@sandersn sandersn added this to Curio in Pall Mall Jan 31, 2020
@sandersn sandersn added this to In progress in PR Backlog Feb 3, 2020
@sandersn sandersn added the For Backlog Bug PRs that fix a backlog bug label Feb 3, 2020
@ExE-Boss
Copy link
Contributor

This will also need fixing in tslib.

@sandersn sandersn removed this from Curio in Pall Mall Mar 9, 2020
@sandersn sandersn moved this from Not started to Needs review in PR Backlog Mar 9, 2020
@sandersn sandersn moved this from Waiting on reviewers to Done in PR Backlog May 24, 2022
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Backlog Bug PRs that fix a backlog bug
Projects
PR Backlog
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants