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

Tracked storage implementations #161

Merged
merged 16 commits into from
Oct 26, 2021
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
2 changes: 1 addition & 1 deletion .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,8 @@ jobs:
matrix:
ember-try-scenario:
[
ember-lts-3.20,
ember-lts-3.24,
ember-lts-3.28,
ember-release,
ember-beta,
]
Expand Down
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,9 @@ used as fully transparent replacements in most circumstances.
Compatibility
------------------------------------------------------------------------------

* Ember.js v3.13 or above
* Ember CLI v2.13 or above
* Node.js v8 or above
* Ember.js v3.24 (LTS) or above
* Ember CLI v3.23 or above
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does it actually require a CLI bump? I'd prefer to just leave CLI alone unless there's a reason to change it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

probably not!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

but, we aren't testing against ember-cli 2.13

* Node.js v12 or above

Contributing
------------------------------------------------------------------------------
Expand Down
170 changes: 87 additions & 83 deletions addon/-private/array.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,11 @@
// that properties within the getter have the correct type in TS.

import {
createTag,
consumeTag,
dirtyTag,
consumeCollection,
dirtyCollection,
} from 'tracked-maps-and-sets/-private/util';

// SEMVER: this relies on the non-user-constructible `Tag` API, which ultimately
// comes from Glimmer itself. In the future, we should rely on a publicly
// exported type for this rather than relying on this inference (as this can
// technically break under us!). For now, this is "safe" for us to absorb
// internally as a "friend" API, but we have to uphold the invariant ourselves.
type Tag = ReturnType<typeof createTag>;
TrackedStorage,
createStorage,
getValue,
setValue,
} from 'ember-tracked-storage-polyfill';

const ARRAY_GETTER_METHODS = new Set<string | symbol | number>([
Symbol.iterator,
Expand Down Expand Up @@ -56,72 +48,6 @@ function convertToInt(prop: number | string | symbol): number | null {
return num % 1 === 0 ? num : null;
}

function createArrayProxy<T>(arr: T[]): TrackedArray<T> {
let indexTags: Tag[] = [];

let boundFns = new Map();

return new Proxy(arr, {
get(target, prop, receiver) {
let index = convertToInt(prop);

if (index !== null) {
let tag = indexTags[index];

if (tag === undefined) {
tag = indexTags[index] = createTag();
}

consumeTag(tag);
consumeCollection(receiver);

return target[index];
} else if (prop === 'length') {
consumeCollection(receiver);
} else if (ARRAY_GETTER_METHODS.has(prop)) {
let fn = boundFns.get(prop);

if (fn === undefined) {
fn = (...args: unknown[]) => {
consumeCollection(receiver);
return (target as any)[prop](...args);
};

boundFns.set(prop, fn);
}

return fn;
}

return (target as any)[prop];
},

set(target, prop, value, receiver) {
(target as any)[prop] = value;

let index = convertToInt(prop);

if (index !== null) {
let tag = indexTags[index];

if (tag !== undefined) {
dirtyTag(tag);
}

dirtyCollection(receiver);
} else if (prop === 'length') {
dirtyCollection(receiver);
}

return true;
},

getPrototypeOf() {
return TrackedArray.prototype;
},
});
}

class TrackedArray<T = unknown> {
/**
* Creates an array from an iterable object.
Expand All @@ -147,16 +73,94 @@ class TrackedArray<T = unknown> {
thisArg?: unknown
): TrackedArray<T> | TrackedArray<U> {
return mapfn
? createArrayProxy(Array.from(iterable, mapfn, thisArg))
: createArrayProxy(Array.from(iterable));
? new TrackedArray(Array.from(iterable, mapfn, thisArg))
: new TrackedArray(Array.from(iterable));
}

static of<T>(...arr: T[]): TrackedArray<T> {
return createArrayProxy(arr);
return new TrackedArray(arr);
}

constructor(arr: T[] = []) {
return createArrayProxy(arr.slice());
let clone = arr.slice();
// eslint-disable-next-line @typescript-eslint/no-this-alias
let self = this;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm using this blast from the past because not all of the proxy hooks have a receiver? or at least my editor didn't say that they did.

If I'm wrong, I'll gladly swap out all the self references to receiver.

Choose a reason for hiding this comment

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

Well, for the array case they all do - because the only hooks needed are get and set. So there you can use receiver. But for the object case you're right.

Choose a reason for hiding this comment

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

Actually no, you can't use receiver as it's the Proxy object, not the instance of the class that has the needed methods.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

that's why let self = this 🙃


let boundFns = new Map();

return new Proxy(clone, {
get(target, prop, _receiver) {
let index = convertToInt(prop);

if (index !== null) {
self.#readStorageFor(index);
getValue(self.#collection)

return target[index];
} else if (prop === 'length') {
getValue(self.#collection)
} else if (ARRAY_GETTER_METHODS.has(prop)) {
let fn = boundFns.get(prop);

if (fn === undefined) {
fn = (...args: unknown[]) => {
getValue(self.#collection)
return (target as any)[prop](...args);
};

boundFns.set(prop, fn);
}

return fn;
}

return (target as any)[prop];
},

set(target, prop, value, _receiver) {
(target as any)[prop] = value;

let index = convertToInt(prop);

if (index !== null) {
self.#dirtyStorageFor(index);
setValue(self.#collection, null)
} else if (prop === 'length') {
setValue(self.#collection, null)
}

return true;
},

getPrototypeOf() {
return TrackedArray.prototype;
},
}) as TrackedArray<T>;
}


#collection = createStorage(null, () => false);

#storages = new Map<number, TrackedStorage<null>>();

#readStorageFor(index: number) {
const storages = this.#storages;
let storage = storages.get(index);

if (storage === undefined) {
storage = createStorage(null, () => false);
storages.set(index, storage);
}

getValue(storage);
}

#dirtyStorageFor(index: number): void {
const storage = this.#storages.get(index);

if (storage) {
setValue(storage, null);
}
}
}

Expand Down
106 changes: 64 additions & 42 deletions addon/-private/object.js
Original file line number Diff line number Diff line change
@@ -1,64 +1,86 @@
import { consumeKey, dirtyKey } from 'tracked-maps-and-sets/-private/util';
import { notifyPropertyChange } from '@ember/object';
import { DEBUG } from '@glimmer/env';
import {
TrackedStorage,
createStorage,
getValue,
setValue,
} from 'ember-tracked-storage-polyfill';

const COLLECTION = Symbol();
export default class TrackedObject {
static fromEntries(entries) {
return new TrackedObject(Object.fromEntries(entries));
}

const proxyHandler = {
get(target, prop) {
consumeKey(target, prop);
constructor(obj = {}) {
let proto = Object.getPrototypeOf(obj);
let descs = Object.getOwnPropertyDescriptors(obj);

return target[prop];
},
let clone = Object.create(proto);

has(target, prop) {
consumeKey(target, prop);
for (let prop in descs) {
Object.defineProperty(clone, prop, descs[prop]);
}

return prop in target;
},
// eslint-disable-next-line @typescript-eslint/no-this-alias
let self = this;

ownKeys(target) {
consumeKey(target, COLLECTION);
return new Proxy(clone, {
get(target, prop, receiver) {
self.#readStorageFor(prop);

return Reflect.ownKeys(target);
},
return target[prop];
},

set(target, prop, value, receiver) {
target[prop] = value;
has(target, prop) {
self.#readStorageFor(prop);

dirtyKey(target, prop);
dirtyKey(target, COLLECTION);
return prop in target;
},

// We need to notify this way to make {{each-in}} update
notifyPropertyChange(receiver, '_SOME_PROP_');
ownKeys(target) {
getValue(self.#collection);

return true;
},
return Reflect.ownKeys(target);
},

getPrototypeOf() {
return TrackedObject.prototype;
},
};
set(target, prop, value, receiver) {
target[prop] = value;

function createProxy(obj = {}) {
return new Proxy(obj, proxyHandler);
}
self.#dirtyStorageFor(prop);
setValue(self.#collection, null);

export default class TrackedObject {
static fromEntries(entries) {
return createProxy(Object.fromEntries(entries));
return true;
},

getPrototypeOf() {
return TrackedObject.prototype;
},
});
}

constructor(obj = {}) {
let proto = Object.getPrototypeOf(obj);
let descs = Object.getOwnPropertyDescriptors(obj);
// @private
#storages = new Map();

let clone = Object.create(proto);
// @private
#collection = createStorage(null, () => false);

for (let prop in descs) {
Object.defineProperty(clone, prop, descs[prop]);
// @private
#readStorageFor(key) {
let storage = this.#storages.get(key);

if (storage === undefined) {
storage = createStorage(null, () => false);
this.#storages.set(key, storage);
}

return createProxy(clone);
getValue(storage);
}

// @private
#dirtyStorageFor(key) {
const storage = this.#storages.get(key);

if (storage) {
setValue(storage, null);
}
}
}
8 changes: 4 additions & 4 deletions config/ember-try.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,18 @@ module.exports = async function() {
useYarn: true,
scenarios: [
{
name: 'ember-lts-3.20',
name: 'ember-lts-3.24',
npm: {
devDependencies: {
'ember-source': '3.20.6',
'ember-source': '3.24.3',
},
},
},
{
name: 'ember-lts-3.24',
name: 'ember-lts-3.28',
npm: {
devDependencies: {
'ember-source': '3.24.3',
'ember-source': '~3.28.0',
},
},
},
Expand Down
9 changes: 5 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@
"postpublish": "ember ts:clean"
},
"dependencies": {
"ember-cli-babel": "^7.26.3",
"ember-cli-babel": "^7.26.6",
"ember-cli-typescript": "^4.1.0",
"tracked-maps-and-sets": "^2.0.0"
"ember-tracked-storage-polyfill": "^1.0.0",
"tracked-maps-and-sets": "^3.0.2"
},
"devDependencies": {
"@ember/optional-features": "^2.0.0",
Expand Down Expand Up @@ -66,11 +67,11 @@
"eslint-plugin-node": "^11.1.0",
"loader.js": "^4.7.0",
"npm-run-all": "^4.1.5",
"qunit": "2.17.2",
"qunit-dom": "^2.0.0",
"release-it": "^14.6.1",
"release-it-lerna-changelog": "^3.1.0",
"typescript": "^4.1.3",
"qunit": "2.17.2"
"typescript": "^4.1.3"
},
"engines": {
"node": "12.* || >= 14.*"
Expand Down
Loading