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.

Sign up for GitHub

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

fix: waiter should be always on #7901

Merged
merged 3 commits into from
Jul 23, 2022
Merged

fix: waiter should be always on #7901

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e5a8b0e
waiter should be always on
runspired Feb 24, 2022
bcc1e97
fix test leak
runspired Jul 23, 2022
f82496f
Remove defer import
runspired Jul 23, 2022
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
12 changes: 6 additions & 6 deletions packages/-ember-data/tests/integration/adapter/queries-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,25 +105,25 @@ module('integration/adapter/queries - Queries', function (hooks) {
assert.false(personsQuery.isUpdating, 'RecordArray is not updating');

let resolveQueryPromise;
const deferred = new Promise((resolve) => {
resolveQueryPromise = resolve;
});

adapter.query = function () {
assert.ok(true, 'query is called a second time');

return new EmberPromise((resolve) => {
resolveQueryPromise = resolve;
});
return deferred;
};

personsQuery.update();

assert.true(personsQuery.isUpdating, 'RecordArray is updating');

// Resolve internal promises to allow the RecordArray to build.
await settled();

resolveQueryPromise({ data: [{ id: 'second', type: 'person' }] });

// Wait for all promises to resolve after the query promise resolves.
// this just ensures that our waiter is waiting, we could also
// wait the return of update.
await settled();

assert.false(personsQuery.isUpdating, 'RecordArray is not updating anymore');
Expand Down
2 changes: 0 additions & 2 deletions packages/-ember-data/tests/integration/store-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,6 @@ module('integration/store - destroy', function (hooks) {

this.owner.register('adapter:application', TestAdapter);

store.shouldTrackAsyncRequests = true;
store.push = function () {
assert('The test should have destroyed the store by now', store.isDestroyed);

Expand Down Expand Up @@ -291,7 +290,6 @@ module('integration/store - findRecord', function (hooks) {
this.owner.register('adapter:application', RESTAdapter.extend());
this.owner.register('serializer:application', RESTSerializer.extend());
store = this.owner.lookup('service:store');
store.shouldTrackAsyncRequests = true;
});

test('store#findRecord fetches record from server when cached record is not present', async function (assert) {
Expand Down
90 changes: 0 additions & 90 deletions packages/-ember-data/tests/unit/store/async-leak-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ module('unit/store async-waiter and leak detection', function (hooks) {
})
);
store = owner.lookup('service:store');
store.shouldTrackAsyncRequests = true;
});

test('the waiter properly waits for pending requests', async function (assert) {
Expand Down Expand Up @@ -71,44 +70,6 @@ module('unit/store async-waiter and leak detection', function (hooks) {
assert.true(waiter(), 'We return true to end waiting when no requests are pending');
});

test('waiter can be turned off', async function (assert) {
let findRecordWasInvoked;
let findRecordWasInvokedPromise = new Promise((resolveStep) => {
findRecordWasInvoked = resolveStep;
});
this.owner.register(
'adapter:application',
JSONAPIAdapter.extend({
findRecord() {
return new Promise((resolve) => {
findRecordWasInvoked();

setTimeout(() => {
resolve({ data: { type: 'person', id: '1' } });
}, 20); // intentionally longer than the 10ms polling interval of `wait()`
});
},
})
);

// turn off the waiter
store.shouldTrackAsyncRequests = false;

let request = store.findRecord('person', '1');
let waiter = store.__asyncWaiter;

assert.true(waiter(), 'We return true when no requests have been initiated yet (pending queue flush is async)');

await findRecordWasInvokedPromise;

assert.strictEqual(store._trackedAsyncRequests.length, 1, 'We return true even though a request is pending');
assert.true(waiter(), 'We return true even though a request is pending');

await request;

assert.true(waiter(), 'We return true to end waiting when no requests are pending');
});

test('waiter works even when the adapter rejects', async function (assert) {
assert.expect(4);
let findRecordWasInvoked;
Expand Down Expand Up @@ -211,57 +172,6 @@ module('unit/store async-waiter and leak detection', function (hooks) {
}
});

test('when the store is torn down too early, but the waiter behavior is turned off, we emit a warning', async function (assert) {
let next;
let stepPromise = new Promise((resolveStep) => {
next = resolveStep;
});
this.owner.register(
'adapter:application',
JSONAPIAdapter.extend({
findRecord() {
next();
stepPromise = new Promise((resolveStep) => {
next = resolveStep;
}).then(() => {
return { data: { type: 'person', id: '1' } };
});
return stepPromise;
},
})
);

// turn off the waiter
store.shouldTrackAsyncRequests = false;

store.findRecord('person', '1');
let waiter = store.__asyncWaiter;

assert.strictEqual(store._trackedAsyncRequests.length, 0, 'We have no requests yet');
assert.true(waiter(), 'We return true when no requests have been initiated yet (pending queue flush is async)');

await stepPromise;

assert.strictEqual(store._trackedAsyncRequests.length, 1, 'We have a pending request');
assert.true(waiter(), 'We return true because the waiter is turned off');
assert.expectWarning(() => {
run(() => {
store.destroy();
});
}, /Async Request leaks detected/);

assert.true(waiter(), 'We return true because the waiter is turned off');

// make the waiter complete
run(() => next());
assert.strictEqual(store._trackedAsyncRequests.length, 0, 'Our pending request is cleaned up');
assert.true(waiter(), 'We return true because the waiter is cleared');

if (DEPRECATE_RSVP_PROMISE) {
assert.expectDeprecation({ id: 'ember-data:rsvp-unresolved-async', count: 1 });
}
});

test('when configured, pending requests have useful stack traces', async function (assert) {
let stepResolve;
let stepPromise = new Promise((resolveStep) => {
Expand Down
29 changes: 5 additions & 24 deletions packages/store/addon/-private/system/core-store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,6 @@ abstract class CoreStore extends Service {

// DEBUG-only properties
declare _trackedAsyncRequests: AsyncTrackingToken[];
declare shouldTrackAsyncRequests: boolean;
declare generateStackTracesForTrackedRequests: boolean;
declare _trackAsyncRequestStart: (str: string) => void;
declare _trackAsyncRequestEnd: (token: AsyncTrackingToken) => void;
Expand Down Expand Up @@ -276,9 +275,6 @@ abstract class CoreStore extends Service {
this.identifierCache = new IdentifierCache();

if (DEBUG) {
if (this.shouldTrackAsyncRequests === undefined) {
this.shouldTrackAsyncRequests = false;
}
if (this.generateStackTracesForTrackedRequests === undefined) {
this.generateStackTracesForTrackedRequests = false;
}
Expand Down Expand Up @@ -317,11 +313,8 @@ abstract class CoreStore extends Service {
};

this.__asyncWaiter = () => {
let shouldTrack = this.shouldTrackAsyncRequests;
let tracked = this._trackedAsyncRequests;
let isSettled = tracked.length === 0;

return shouldTrack !== true || isSettled;
return tracked.length === 0;
};

registerWaiter(this.__asyncWaiter);
Expand Down Expand Up @@ -3185,26 +3178,14 @@ abstract class CoreStore extends Service {

if (DEBUG) {
unregisterWaiter(this.__asyncWaiter);
let shouldTrack = this.shouldTrackAsyncRequests;
let tracked = this._trackedAsyncRequests;
let isSettled = tracked.length === 0;

if (!isSettled) {
if (shouldTrack) {
throw new Error(
'Async Request leaks detected. Add a breakpoint here and set `store.generateStackTracesForTrackedRequests = true;`to inspect traces for leak origins:\n\t - ' +
tracked.map((o) => o.label).join('\n\t - ')
);
} else {
warn(
'Async Request leaks detected. Add a breakpoint here and set `store.generateStackTracesForTrackedRequests = true;`to inspect traces for leak origins:\n\t - ' +
tracked.map((o) => o.label).join('\n\t - '),
false,
{
id: 'ds.async.leak.detected',
}
);
}
throw new Error(
'Async Request leaks detected. Add a breakpoint here and set `store.generateStackTracesForTrackedRequests = true;`to inspect traces for leak origins:\n\t - ' +
tracked.map((o) => o.label).join('\n\t - ')
);
}
}
}
Expand Down