Skip to content

Commit bdf7ddd

Browse files
committed
crate: findRecord requests for crate without including versions by default
This will ensure that the `versions` are fetched via a separate requests, rather than being included in the crate's response. This not only enables us to render the page layout first for a faster first paint but also allows us to migrate to a paginated version list in the future.
1 parent c4e851b commit bdf7ddd

File tree

7 files changed

+41
-13
lines changed

7 files changed

+41
-13
lines changed

app/adapters/crate.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,15 @@ const BULK_REQUEST_GROUP_SIZE = 10;
55
export default class CrateAdapter extends ApplicationAdapter {
66
coalesceFindRequests = true;
77

8+
findRecord(store, type, id, snapshot) {
9+
let { include } = snapshot;
10+
// This ensures `crate.versions` are always fetched from another request.
11+
if (include === undefined) {
12+
snapshot.include = 'keywords,categories,downloads';
13+
}
14+
return super.findRecord(store, type, id, snapshot);
15+
}
16+
817
groupRecordsForFindMany(store, snapshots) {
918
let result = [];
1019
for (let i = 0; i < snapshots.length; i += BULK_REQUEST_GROUP_SIZE) {

e2e/acceptance/crate-dependencies.spec.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,16 +76,17 @@ test.describe('Acceptance | crate dependencies page', { tag: '@acceptance' }, ()
7676
});
7777

7878
await ember.addHook(async owner => {
79-
// Load `crate` and then explicitly unload the side-loaded `versions`.
79+
// Load `crate` and ensure all `versions` are not loaded.
8080
let store = owner.lookup('service:store');
8181
let crateRecord = await store.findRecord('crate', 'foo');
82-
let versions = crateRecord.hasMany('versions').value();
83-
versions.forEach(record => record.unloadRecord());
82+
let versionsRef = crateRecord.hasMany('versions');
83+
globalThis.version_ids = versionsRef.ids();
8484
});
8585

8686
await page.goto('/crates/foo/1.0.0/dependencies');
8787

8888
await expect(page).toHaveURL('/crates/foo/1.0.0/dependencies');
89+
await page.waitForFunction(() => globalThis.version_ids?.length === 0);
8990
await expect(page.locator('[data-test-404-page]')).toBeVisible();
9091
await expect(page.locator('[data-test-title]')).toHaveText('foo: Failed to load version data');
9192
await expect(page.locator('[data-test-go-back]')).toHaveCount(0);

e2e/routes/crate/range.spec.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,15 +128,16 @@ test.describe('Route | crate.range', { tag: '@routes' }, () => {
128128
});
129129

130130
await ember.addHook(async owner => {
131-
// Load `crate` and then explicitly unload the side-loaded `versions`.
131+
// Load `crate` and ensure all `versions` are not loaded.
132132
let store = owner.lookup('service:store');
133133
let crateRecord = await store.findRecord('crate', 'foo');
134-
let versions = crateRecord.hasMany('versions').value();
135-
versions.forEach(record => record.unloadRecord());
134+
let versionsRef = crateRecord.hasMany('versions');
135+
globalThis.version_ids = versionsRef.ids();
136136
});
137137

138138
await page.goto('/crates/foo/range/^3');
139139
await expect(page).toHaveURL('/crates/foo/range/%5E3');
140+
await page.waitForFunction(() => globalThis.version_ids?.length === 0);
140141
await expect(page.locator('[data-test-404-page]')).toBeVisible();
141142
await expect(page.locator('[data-test-title]')).toHaveText('foo: Failed to load version data');
142143
await expect(page.locator('[data-test-go-back]')).toHaveCount(0);

tests/acceptance/crate-dependencies-test.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,11 @@ module('Acceptance | crate dependencies page', function (hooks) {
7878

7979
this.server.get('/api/v1/crates/:crate_name/versions', {}, 500);
8080

81-
// Load `crate` and then explicitly unload the side-loaded `versions`.
81+
// Load `crate` and ensure all `versions` are not loaded.
8282
let store = this.owner.lookup('service:store');
8383
let crateRecord = await store.findRecord('crate', 'foo');
84-
let versions = crateRecord.hasMany('versions').value();
85-
versions.forEach(record => record.unloadRecord());
84+
let versionsRef = crateRecord.hasMany('versions');
85+
assert.deepEqual(versionsRef.ids(), []);
8686

8787
await visit('/crates/foo/1.0.0/dependencies');
8888
assert.strictEqual(currentURL(), '/crates/foo/1.0.0/dependencies');

tests/adapters/crate-test.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,21 @@ module('Adapter | crate', function (hooks) {
2222
assert.strictEqual(foo?.name, 'foo');
2323
assert.strictEqual(bar?.name, 'bar');
2424
});
25+
26+
test('findRecord requests do not include versions by default', async function (assert) {
27+
let _foo = this.server.create('crate', { name: 'foo' });
28+
let version = this.server.create('version', { crate: _foo });
29+
30+
let store = this.owner.lookup('service:store');
31+
32+
let foo = await store.findRecord('crate', 'foo');
33+
assert.strictEqual(foo?.name, 'foo');
34+
35+
// versions should not be loaded yet
36+
let versionsRef = foo.hasMany('versions');
37+
assert.deepEqual(versionsRef.ids(), []);
38+
39+
await versionsRef.load();
40+
assert.deepEqual(versionsRef.ids(), [version.id]);
41+
});
2542
});

tests/models/version-test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ module('Model | Version', function (hooks) {
229229
this.server.create('version', { crate, num: '0.4.2' });
230230
this.server.create('version', { crate, num: '0.4.3', yanked: true });
231231
crateRecord = await this.store.findRecord('crate', crate.name, { reload: true });
232-
versions = (await crateRecord.loadVersionsTask.perform()).slice();
232+
versions = (await crateRecord.loadVersionsTask.perform({ reload: true })).slice();
233233

234234
assert.deepEqual(
235235
versions.map(it => ({ num: it.num, isHighestOfReleaseTrack: it.isHighestOfReleaseTrack })),

tests/routes/crate/range-test.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,11 +119,11 @@ module('Route | crate.range', function (hooks) {
119119

120120
this.server.get('/api/v1/crates/:crate_name/versions', {}, 500);
121121

122-
// Load `crate` and then explicitly unload the side-loaded `versions`.
122+
// Load `crate` and ensure all `versions` are not loaded.
123123
let store = this.owner.lookup('service:store');
124124
let crateRecord = await store.findRecord('crate', 'foo');
125-
let versions = crateRecord.hasMany('versions').value();
126-
versions.forEach(record => record.unloadRecord());
125+
let versionsRef = crateRecord.hasMany('versions');
126+
assert.deepEqual(versionsRef.ids(), []);
127127

128128
await visit('/crates/foo/range/^3');
129129
assert.strictEqual(currentURL(), '/crates/foo/range/%5E3');

0 commit comments

Comments
 (0)