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

[BUGFIX beta] Use native Map if present. #5255

Merged
merged 1 commit into from
Mar 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ module.exports = {
'browser': true,
},
globals: {
'heimdall': true
'heimdall': true,
'Map': false,
},
rules: {
'no-unused-vars': ['error', {
Expand Down
3 changes: 3 additions & 0 deletions addon/-private/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ export { default as ManyArray } from './system/many-array';
export { default as RecordArrayManager } from './system/record-array-manager';
export { default as Relationship } from './system/relationships/state/relationship';

export { default as Map } from './system/map';
export { default as MapWithDefault } from './system/map-with-default';

// Should be a different Repo ?
export { default as DebugAdapter } from './system/debug/debug-adapter';

Expand Down
21 changes: 21 additions & 0 deletions addon/-private/system/map-with-default.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import Map from './map';

export default class MapWithDefault extends Map {
constructor(options) {
super();

this.defaultValue = options.defaultValue;
}

get(key) {
let hasValue = this.has(key);

if (hasValue) {
return super.get(key);
} else {
let defaultValue = this.defaultValue(key);
this.set(key, defaultValue);
return defaultValue;
}
}
}
102 changes: 102 additions & 0 deletions addon/-private/system/map.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
import { deprecate } from '@ember/debug';

/*
## Why does this exist?!?

`Ember.Map` was a private API provided by Ember (for quite some time).
Unfortunately, ember-data made `Ember.Map` part of its public API surface via
documentation blocks.

`Ember.Map` will be deprecated and removed from Ember "soon"
(https://github.com/emberjs/rfcs/pull/237) and we would like to confirm that
Ember Data will work without deprecation before and after that happens.

`Ember.Map` differs from native `Map` in a few ways:

* `Ember.Map` has custom `copy` and `isEmpty` methods which are not present in native `Map`
* `Ember.Map` adds a static `create` method (which simply instantiates itself with `new Ember.Map()`)
* `Ember.Map` does not accept constructor arguments
* `Ember.Map` does not have:
* `@@species`
* `@@iterator`
* `entries`
* `values`

This implementation adds a deprecated backwards compatibility for:

* `copy`
* `isEmpty`

## Why is this written this way?!?

This is needed because `Map` requires instantiation with `new` and by default
Babel transpilation will do `superConstructor.apply(this, arguments)` which
throws an error with native `Map`.

The desired code (if we lived in an "only native class" world) would be:

```js
export default class MapWithDeprecations extends Map {
constructor(options) {
super();
this.defaultValue = options.defaultValue;
}

get(key) {
let hasValue = this.has(key);

if (hasValue) {
return super.get(key);
} else {
let defaultValue = this.defaultValue(key);
this.set(key, defaultValue);
return defaultValue;
}
}
}
```
*/
export default class MapWithDeprecations {
constructor(options) {
this._map = new Map();
}

copy() {
deprecate(
'Calling `.copy()` on a map generated by ember-data is deprecated, please migrate to using native Map functionality only.',
false,
{ id: 'ember-data.map.copy', until: '3.5.0' }
);

// can't just pass `this._map` here because IE11 doesn't accept
// constructor args with its `Map`
let newMap = new MapWithDeprecations();
this._map.forEach(function(value, key) {
newMap.set(key, value)
});

return newMap;
}

isEmpty() {
deprecate(
'Calling `.isEmpty()` on a map generated by ember-data is deprecated, please migrate to using native Map functionality only.',
false,
{ id: 'ember-data.map.isEmpty', until: '3.5.0' }
);

return this.size === 0;
}

// proxy all normal Map methods to the underlying Map
get size() { return this._map.size; }
clear() { return this._map.clear(...arguments) }
delete() { return this._map.delete(...arguments) }
entries() { return this._map.entries(...arguments) }
forEach() { return this._map.forEach(...arguments) }
get() { return this._map.get(...arguments) }
has() { return this._map.has(...arguments) }
keys() { return this._map.keys(...arguments) }
set() { return this._map.set(...arguments) }
values() { return this._map.values(...arguments) }
}
6 changes: 3 additions & 3 deletions addon/-private/system/model/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import ArrayProxy from '@ember/array/proxy';
import { set, get, computed } from '@ember/object';
import { isEmpty } from '@ember/utils';
import { makeArray, A } from '@ember/array';
import MapWithDefault from '@ember/map/with-default';
import MapWithDefault from '../map-with-default';
import { deprecate, warn } from '@ember/debug';

/**
Expand Down Expand Up @@ -121,11 +121,11 @@ export default ArrayProxy.extend(Evented, {

/**
@property errorsByAttributeName
@type {Ember.MapWithDefault}
@type {MapWithDefault}
@private
*/
errorsByAttributeName: computed(function() {
return MapWithDefault.create({
return new MapWithDefault({
defaultValue() {
return A();
}
Expand Down
18 changes: 9 additions & 9 deletions addon/-private/system/model/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import EmberObject, {
get,
observer
} from '@ember/object';
import Map from '@ember/map';
import Map from '../map';
import { DEBUG } from '@glimmer/env';
import { assert, warn } from '@ember/debug';
import { PromiseObject } from "../promise-proxies";
Expand Down Expand Up @@ -1399,7 +1399,7 @@ Model.reopenClass({

@property relationships
@static
@type Ember.Map
@type Map
@readOnly
*/

Expand Down Expand Up @@ -1522,7 +1522,7 @@ Model.reopenClass({

@property relationshipsByName
@static
@type Ember.Map
@type Map
@readOnly
*/
relationshipsByName: relationshipsByNameDescriptor,
Expand Down Expand Up @@ -1565,11 +1565,11 @@ Model.reopenClass({

@property fields
@static
@type Ember.Map
@type Map
@readOnly
*/
fields: computed(function() {
let map = Map.create();
let map = new Map();

this.eachComputedProperty((name, meta) => {
if (meta.isRelationship) {
Expand Down Expand Up @@ -1674,11 +1674,11 @@ Model.reopenClass({

@property attributes
@static
@type {Ember.Map}
@type {Map}
@readOnly
*/
attributes: computed(function() {
let map = Map.create();
let map = new Map();

this.eachComputedProperty((name, meta) => {
if (meta.isAttribute) {
Expand Down Expand Up @@ -1727,11 +1727,11 @@ Model.reopenClass({

@property transformedAttributes
@static
@type {Ember.Map}
@type {Map}
@readOnly
*/
transformedAttributes: computed(function() {
let map = Map.create();
let map = new Map();

this.eachAttribute((key, meta) => {
if (meta.type) {
Expand Down
6 changes: 3 additions & 3 deletions addon/-private/system/relationships/ext.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { A } from '@ember/array';
import { computed } from '@ember/object';
import MapWithDefault from '@ember/map/with-default';
import Map from '@ember/map';
import MapWithDefault from '../map-with-default';
import Map from '../map';
import { assert } from '@ember/debug';
import {
typeForRelationshipMeta,
Expand Down Expand Up @@ -56,7 +56,7 @@ export const relatedTypesDescriptor = computed(function() {
}).readOnly();

export const relationshipsByNameDescriptor = computed(function() {
let map = Map.create();
let map = new Map();

this.eachComputedProperty((name, meta) => {
if (meta.isRelationship) {
Expand Down
4 changes: 2 additions & 2 deletions addon/-private/system/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { A } from '@ember/array';

import { copy } from '@ember/object/internals';
import EmberError from '@ember/error';
import MapWithDefault from '@ember/map/with-default';
import MapWithDefault from './map-with-default';
import { run as emberRun } from '@ember/runloop';
import { set, get, computed } from '@ember/object';
import RSVP from 'rsvp';
Expand Down Expand Up @@ -230,7 +230,7 @@ Store = Service.extend({
this._updatedInternalModels = [];

// used to keep track of all the find requests that need to be coalesced
this._pendingFetch = MapWithDefault.create({ defaultValue() { return []; } });
this._pendingFetch = new MapWithDefault({ defaultValue() { return []; } });

this._adapterCache = Object.create(null);
this._serializerCache = Object.create(null);
Expand Down
6 changes: 3 additions & 3 deletions addon/adapters/rest.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import $ from 'jquery';

import { Promise as EmberPromise } from 'rsvp';
import MapWithDefault from '@ember/map/with-default';
import { get } from '@ember/object';
import { run } from '@ember/runloop';
import Adapter from "../adapter";
Expand All @@ -22,7 +21,8 @@ import {
ConflictError,
ServerError,
TimeoutError,
AbortError
AbortError,
MapWithDefault
} from '../-private';
import { instrument } from 'ember-data/-debug';
import { warn, deprecate } from '@ember/debug';
Expand Down Expand Up @@ -901,7 +901,7 @@ const RESTAdapter = Adapter.extend(BuildURLMixin, {
loaded separately by `findMany`.
*/
groupRecordsForFindMany(store, snapshots) {
let groups = MapWithDefault.create({ defaultValue() { return []; } });
let groups = new MapWithDefault({ defaultValue() { return []; } });
let adapter = this;
let maxURLLength = this.maxURLLength;

Expand Down