Skip to content

Commit

Permalink
[cleanup]: Remove mutation after consumption autotracking.mutation-af…
Browse files Browse the repository at this point in the history 
…ter-consumption
  • Loading branch information
@snewcomer
snewcomer committed Sep 11, 2021
1 parent 56f5c1e commit e6af391
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 152 deletions.
11 changes: 1 addition & 10 deletions packages/@glimmer/manager/lib/public/component.ts
View file
Expand Up @@ -18,7 +18,6 @@ import {
} from '@glimmer/interfaces';
import { createConstRef, Reference } from '@glimmer/reference';
import { registerDestructor } from '@glimmer/destroyable';
import { deprecateMutationsInTrackingTransaction } from '@glimmer/validator';
import { buildCapabilities, FROM_CAPABILITIES } from '../util/capabilities';
import { argsProxyFor } from '../util/args-proxy';
import { ManagerFactory } from './index';
Expand Down Expand Up @@ -142,15 +141,7 @@ export class CustomComponentManager<O extends Owner, ComponentInstance>
let delegate = this.getDelegateFor(owner);
let args = argsProxyFor(vmArgs.capture(), 'component');

let component: ComponentInstance;

if (DEBUG && deprecateMutationsInTrackingTransaction !== undefined) {
deprecateMutationsInTrackingTransaction(() => {
component = delegate.createComponent(definition, args);
});
} else {
component = delegate.createComponent(definition, args);
}
let component: ComponentInstance = delegate.createComponent(definition, args);

return new CustomComponentState(component!, delegate, args);
}
Expand Down
17 changes: 2 additions & 15 deletions packages/@glimmer/manager/lib/public/modifier.ts
View file
Expand Up @@ -12,12 +12,7 @@ import { registerDestructor } from '@glimmer/destroyable';
import { setOwner } from '@glimmer/owner';
import { valueForRef } from '@glimmer/reference';
import { assign, castToBrowser, dict } from '@glimmer/util';
import {
createUpdatableTag,
deprecateMutationsInTrackingTransaction,
untrack,
UpdatableTag,
} from '@glimmer/validator';
import { createUpdatableTag, untrack, UpdatableTag } from '@glimmer/validator';
import { SimpleElement } from '@simple-dom/interface';
import { buildCapabilities, FROM_CAPABILITIES } from '../util/capabilities';
import { argsProxyFor } from '../util/args-proxy';
Expand Down Expand Up @@ -115,8 +110,6 @@ export class CustomModifierManager<O extends Owner, ModifierInstance>
let argsProxy = argsProxyFor(capturedArgs, 'modifier');
let args = useArgsProxy ? argsProxy : reifyArgs(capturedArgs);

let instance: ModifierInstance;

let factoryOrDefinition = definition;

if (passFactoryToCreate) {
Expand All @@ -134,13 +127,7 @@ export class CustomModifierManager<O extends Owner, ModifierInstance>
};
}

if (DEBUG && deprecateMutationsInTrackingTransaction !== undefined) {
deprecateMutationsInTrackingTransaction(() => {
instance = delegate.createModifier(factoryOrDefinition, args);
});
} else {
instance = delegate.createModifier(factoryOrDefinition, args);
}
let instance: ModifierInstance = delegate.createModifier(factoryOrDefinition, args);

let tag = createUpdatableTag();
let state: CustomModifierState<ModifierInstance>;
Expand Down
1 change: 0 additions & 1 deletion packages/@glimmer/validator/index.ts
View file
Expand Up @@ -67,5 +67,4 @@ export {
runInTrackingTransaction,
beginTrackingTransaction,
endTrackingTransaction,
deprecateMutationsInTrackingTransaction,
} from './lib/debug';
66 changes: 17 additions & 49 deletions packages/@glimmer/validator/lib/debug.ts
View file
@@ -1,6 +1,6 @@
import { Tag } from './validators';
import { DEBUG } from '@glimmer/env';
import { deprecate, assert } from '@glimmer/global-context';
import { assert } from '@glimmer/global-context';

export let beginTrackingTransaction:
| undefined
Expand All @@ -9,7 +9,6 @@ export let endTrackingTransaction: undefined | (() => void);
export let runInTrackingTransaction:
| undefined
| (<T>(fn: () => T, debuggingContext?: string | false) => T);
export let deprecateMutationsInTrackingTransaction: undefined | ((fn: () => void) => void);

export let resetTrackingTransaction: undefined | (() => string);
export let setTrackingTransactionEnv:
Expand All @@ -27,7 +26,6 @@ export let logTrackingStack: undefined | ((transaction?: Transaction) => string)
interface Transaction {
parent: Transaction | null;
debugLabel?: string;
deprecate: boolean;
}

if (DEBUG) {
Expand Down Expand Up @@ -61,7 +59,7 @@ if (DEBUG) {

setTrackingTransactionEnv = (env) => Object.assign(TRANSACTION_ENV, env);

beginTrackingTransaction = (_debugLabel?: string | false, deprecate = false) => {
beginTrackingTransaction = (_debugLabel?: string | false) => {
CONSUMED_TAGS = CONSUMED_TAGS || new WeakMap();

let debugLabel = _debugLabel || undefined;
Expand All @@ -71,7 +69,6 @@ if (DEBUG) {
TRANSACTION_STACK.push({
parent,
debugLabel,
deprecate,
});
};

Expand Down Expand Up @@ -125,27 +122,6 @@ if (DEBUG) {
}
};

/**
* Switches to deprecating within an autotracking transaction, if one exists.
* If `runInAutotrackingTransaction` is called within the callback of this
* method, it switches back to throwing an error, allowing zebra-striping of
* the types of errors that are thrown.
*
* Does not start an autotracking transaction.
*
* NOTE: For Ember usage only, in general you should assert that these
* invariants are true.
*/
deprecateMutationsInTrackingTransaction = (fn: () => void, debugLabel?: string | false) => {
beginTrackingTransaction!(debugLabel, true);

try {
fn();
} finally {
endTrackingTransaction!();
}
};

let nthIndex = (str: string, pattern: string, n: number, startingPos = -1) => {
let i = startingPos;

Expand Down Expand Up @@ -218,31 +194,23 @@ if (DEBUG) {

if (!transaction) return;

let currentTransaction = TRANSACTION_STACK[TRANSACTION_STACK.length - 1];

if (currentTransaction.deprecate) {
deprecate(makeTrackingErrorMessage(transaction, obj, keyName), false, {
id: 'autotracking.mutation-after-consumption',
});
} else {
// This hack makes the assertion message nicer, we can cut off the first
// few lines of the stack trace and let users know where the actual error
// occurred.
try {
assert(false, makeTrackingErrorMessage(transaction, obj, keyName));
} catch (e) {
if (e.stack) {
let updateStackBegin = e.stack.indexOf('Stack trace for the update:');

if (updateStackBegin !== -1) {
let start = nthIndex(e.stack, '\n', 1, updateStackBegin);
let end = nthIndex(e.stack, '\n', 4, updateStackBegin);
e.stack = e.stack.substr(0, start) + e.stack.substr(end);
}
// This hack makes the assertion message nicer, we can cut off the first
// few lines of the stack trace and let users know where the actual error
// occurred.
try {
assert(false, makeTrackingErrorMessage(transaction, obj, keyName));
} catch (e) {
if (e.stack) {
let updateStackBegin = e.stack.indexOf('Stack trace for the update:');

if (updateStackBegin !== -1) {
let start = nthIndex(e.stack, '\n', 1, updateStackBegin);
let end = nthIndex(e.stack, '\n', 4, updateStackBegin);
e.stack = e.stack.substr(0, start) + e.stack.substr(end);
}

throw e;
}

throw e;
}
};
}
77 changes: 0 additions & 77 deletions packages/@glimmer/validator/test/tracking-test.ts
View file
Expand Up @@ -7,12 +7,9 @@ import {
createTag,
beginTrackFrame,
endTrackFrame,
deprecateMutationsInTrackingTransaction,
dirtyTag,
dirtyTagFor,
isTracking,
runInTrackingTransaction,
tagFor,
track,
trackedData,
untrack,
Expand Down Expand Up @@ -495,32 +492,6 @@ module('@glimmer/validator: tracking', () => {
});
}, /You attempted to update `foo` on `\(an instance of/);
});

test('it can switches to warning/deprecations when attempting to update a value already consumed in the same transaction', (assert) => {
class Foo {
foo = 123;
bar = 456;
}

let { getter, setter } = trackedData<Foo, keyof Foo>('foo', function (this: Foo) {
return this.bar;
});

let foo = new Foo();

runInTrackingTransaction!(() => {
track(() => {
deprecateMutationsInTrackingTransaction!(() => {
getter(foo);
setter(foo, 789);
});
});
});

assert.validateDeprecations(
/You attempted to update `foo` on `.*`, but it had already been used previously in the same computation/
);
});
}
});

Expand Down Expand Up @@ -585,54 +556,6 @@ module('@glimmer/validator: tracking', () => {
});
}, /Error: You attempted to update `\(an unknown tag\)`/);
});

test('it can switch to warnings/deprecations', (assert) => {
let tag = createTag();

runInTrackingTransaction!(() => {
track(() => {
deprecateMutationsInTrackingTransaction!(() => {
consumeTag(tag);
dirtyTag(tag);
});
});
});

assert.validateDeprecations(
/You attempted to update `.*`, but it had already been used previously in the same computation./
);
});

test('it switches back to errors with nested track calls', (assert) => {
let tag = createTag();

assert.throws(() => {
runInTrackingTransaction!(() => {
deprecateMutationsInTrackingTransaction!(() => {
track(() => {
consumeTag(tag);
dirtyTag(tag);
});
});
});
}, /Error: You attempted to update `\(an unknown tag\)`/);
});

test('it gets a better error message with tagFor', (assert) => {
class Foo {}
let foo = new Foo();

assert.throws(() => {
runInTrackingTransaction!(() => {
deprecateMutationsInTrackingTransaction!(() => {
track(() => {
consumeTag(tagFor(foo, 'bar'));
dirtyTagFor(foo, 'bar');
});
});
});
}, /Error: You attempted to update `bar` on `\(an instance of .*\)`/);
});
});
}
});

0 comments on commit e6af391

Please sign in to comment.