-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Leverage native WeakMaps #15878
Leverage native WeakMaps #15878
Conversation
Since beta is 2.18, the commit message should either be |
Done. I liked cleanup better, helps in reading commit messages on any branch 😄 |
packages/ember-metal/lib/meta.js
Outdated
|
||
setMeta = function WeakMap_setMeta(obj, meta) { | ||
let setMeta = function WeakMap_setMeta(obj, meta) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we no longer need to swap based on environment, lets clean things up a bit and make these:
export function setMeta(obj, meta) {
// ...snip...
}
export function peekMeta(obj, meta) {
// ...snip...
}
Then we can remove the export { setMeta, peekMeta }
at the end of the file too...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Its our good old friend,
|
https://saucelabs.com/beta/tests/ae05d4adcf3245e4a2fb7065cb235b40/jsunit doesn’t seem to have test details. I'll run these against browserstack to hopefully get more details. |
@sivakumar-kailasam This is most likely because Window.WeakMap does not exist on IE 9/10. Since 3.0 will be dropping support for these we should probably not test against them. |
This issue should be fixed with: #15879 |
Merged, can you rebase? |
@@ -139,9 +138,6 @@ Ember.bind = metal.bind; | |||
Ember.Binding = metal.Binding; | |||
Ember.isGlobalPath = metal.isGlobalPath; | |||
|
|||
if (EMBER_METAL_WEAKMAP) { | |||
Ember.WeakMap = metal.WeakMap; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be put behind a deprecate method for the remainder of 2.X?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably. @rwjblue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thoov - The feature flag was never enabled, so it has never been published and usable by users. AFAIK all users of this have to go through ember-weakmap
which detects if Ember.WeakMap
is set or else defines it...
Part of #15876