Skip to content

[CLEANUP] Cleanup IE11 leftovers#19575

Closed
simonihmig wants to merge 5 commits intoemberjs:v4-cleanupfrom
simonihmig:ie-cleanup
Closed

[CLEANUP] Cleanup IE11 leftovers#19575
simonihmig wants to merge 5 commits intoemberjs:v4-cleanupfrom
simonihmig:ie-cleanup

Conversation

@simonihmig
Copy link
Contributor

@simonihmig simonihmig commented May 28, 2021

Further cleanup after dropping IE11 support: (best reviewed commit after commit)

  • Use WeakSet for lazy events
  • Remove Object.entries/.values polyfills
  • Remove HAS_NATIVE_SYMBOL and HAS_NATIVE_PROXY flags (as all supported browser support these)

After dropping IE11 support, all supported browsers support symbols natively.
After dropping IE11 support, all supported browsers support proxies natively.
@nlfurniss
Copy link
Contributor

nlfurniss commented May 29, 2021

@simonihmig small correction to the description: Remove Object.assign/.value polyfills --> Remove Object.entries/.values polyfills

@simonihmig
Copy link
Contributor Author

Oh, right, corrected. Thanks!

Copy link
Member

@mixonic mixonic left a comment

Choose a reason for hiding this comment

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

Very cool. This might have been easier as a distinct PR for each change, but I'm mostly comfortable with it after scrolling through it.

Left two notes to get clarity on.

import { extractRouteArgs, resemblesURL, shallowEqual } from '../utils';

const ROUTER = symbol('ROUTER') as string;
const ROUTER = (symbol('ROUTER') as unknown) as string;
Copy link
Member

Choose a reason for hiding this comment

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

What is the motivation for this change?

version "7.12.1"
resolved "https://registry.yarnpkg.com/@babel/plugin-transform-for-of/-/plugin-transform-for-of-7.12.1.tgz#07640f28867ed16f9511c99c888291f560921cfa"
integrity sha512-Zaeq10naAsuHo7heQvyV0ptj4dlZJwZgNAtBYBnu5nNKJoW62m0zKcIEyVECrUKErkUkg6ajMy4ZfnVZciSBhg==
dependencies:
Copy link
Member

Choose a reason for hiding this comment

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

Should there be yarn.lock changes in this PR? I think not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably somehow by accident, removed that in my reopened PR!

@mixonic mixonic mentioned this pull request Jul 18, 2021
58 tasks
@rwjblue rwjblue closed this Jul 18, 2021
@rwjblue rwjblue deleted the branch emberjs:v4-cleanup July 18, 2021 02:05
@rwjblue
Copy link
Member

rwjblue commented Jul 18, 2021

Bah, I thought this would just update to point at master :(

@mixonic
Copy link
Member

mixonic commented Jul 18, 2021

@simonihmig this PR was auto-closed when v4-cleanup was merged to master. Can you re-open for us to review/merge against master? This is excellent work and I don't want to lose it.

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.

4 participants