-
Notifications
You must be signed in to change notification settings - Fork 28
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
Changes from 15 commits
463e969
db6d69c
a1e79ff
e16c492
de6a07c
66af90e
7a45111
ccb6753
fea805d
5edc87e
924bfa8
504231d
85fa6f3
4f7c44d
a14590d
abe972f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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, | ||
|
@@ -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. | ||
|
@@ -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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually no, you can't use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that's why |
||
|
||
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: Map<number, TrackedStorage<null>> = new Map(); | ||
chriskrycho marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
#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); | ||
} | ||
} | ||
} | ||
|
||
|
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); | ||
} | ||
} | ||
} |
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.
Does it actually require a CLI bump? I'd prefer to just leave CLI alone unless there's a reason to change it.
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 not!
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.
but, we aren't testing against ember-cli 2.13