Skip to content

Commit

Permalink
fix(cache): optmize context clean up for observed property
Browse files Browse the repository at this point in the history
  • Loading branch information
smalluban committed Jan 27, 2020
1 parent e2dd413 commit 1ad1bb0
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 17 deletions.
31 changes: 17 additions & 14 deletions src/cache.js
Expand Up @@ -49,7 +49,6 @@ export function get(target, key, getter) {
const entry = getEntry(target, key);

if (contextStack.size && contextStack.has(entry)) {
contextStack.clear();
throw Error(`Circular get invocation of the '${key}' property in '${stringifyElement(target)}'`);
}

Expand All @@ -63,21 +62,20 @@ export function get(target, key, getter) {
}
});

contextStack.add(entry);

if (entry.checksum && entry.checksum === calculateChecksum(entry)) {
contextStack.delete(entry);
return entry.value;
}

if (entry.deps && entry.deps.size) {
entry.deps.forEach((depEntry) => {
if (depEntry.contexts) depEntry.contexts.delete(entry);
});
entry.deps = undefined;
}

try {
contextStack.add(entry);

if (entry.observed && entry.deps && entry.deps.size) {
entry.deps.forEach((depEntry) => {
depEntry.contexts.delete(entry);
});
}

entry.deps = undefined;
const nextValue = getter(target, entry.value);

if (nextValue !== entry.value) {
Expand All @@ -90,7 +88,14 @@ export function get(target, key, getter) {
entry.checksum = calculateChecksum(entry);
contextStack.delete(entry);
} catch (e) {
contextStack.clear();
entry.checksum = 0;

contextStack.delete(entry);
contextStack.forEach((context) => {
context.deps.delete(entry);
if (context.observed) entry.contexts.delete(context);
});

throw e;
}

Expand All @@ -99,7 +104,6 @@ export function get(target, key, getter) {

export function set(target, key, setter, value, force) {
if (contextStack.size && !force) {
contextStack.clear();
throw Error(`Try to set '${key}' of '${stringifyElement(target)}' in get call`);
}

Expand All @@ -117,7 +121,6 @@ export function set(target, key, setter, value, force) {

export function invalidate(target, key, clearValue) {
if (contextStack.size) {
contextStack.clear();
throw Error(`Try to invalidate '${key}' in '${stringifyElement(target)}' get call`);
}

Expand Down
2 changes: 1 addition & 1 deletion src/define.js
Expand Up @@ -82,7 +82,7 @@ if (process.env.NODE_ENV !== 'production') {
node.disconnectedCallback();

Object.keys(node.constructor.hybrids).forEach((key) => {
cache.invalidate(node, key, node[key] === hybrids[key]);
cache.invalidate(node, key, node.constructor.hybrids[key] !== hybrids[key]);
});

node.connectedCallback();
Expand Down
16 changes: 14 additions & 2 deletions test/spec/cache.js
Expand Up @@ -152,7 +152,7 @@ describe('cache:', () => {
});
});

it('clean emitter when unobserve', (done) => {
it('cleans emitter when unobserve', (done) => {
const unobserve = observe(target, 'key', _, spy);

requestAnimationFrame(() => {
Expand All @@ -165,7 +165,7 @@ describe('cache:', () => {
});
});

it('clean dependencies contexts when unobserve', (done) => {
it('cleans dependencies contexts when unobserve', (done) => {
const getter = () => get(target, 'otherKey', () => get(target, 'deepKey', _));
const unobserve = observe(target, 'key', getter, spy);

Expand All @@ -179,5 +179,17 @@ describe('cache:', () => {
});
});
});

it('cleans contexts when getter throws', (done) => {
const getKey = () => get(target, 'key', () => get(target, 'otherKey', () => { throw Error(); }));
const unobserve = observe(target, 'key', () => {}, spy);

expect(() => getKey()).toThrow();

requestAnimationFrame(() => {
expect(() => unobserve()).not.toThrow();
done();
});
});
});
});

0 comments on commit 1ad1bb0

Please sign in to comment.