Skip to content

Commit

Permalink
crate: findRecord requests for crate without including versions
Browse files Browse the repository at this point in the history
… 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.
  • Loading branch information
eth3lbert committed Dec 31, 2024
1 parent c4e851b commit 698a82c
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 29 deletions.
9 changes: 9 additions & 0 deletions app/adapters/crate.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,15 @@ const BULK_REQUEST_GROUP_SIZE = 10;
export default class CrateAdapter extends ApplicationAdapter {
coalesceFindRequests = true;

findRecord(store, type, id, snapshot) {
let { include } = snapshot;
// This ensures `crate.versions` are always fetched from another request.
if (include === undefined) {
snapshot.include = 'keywords,categories,downloads';
}
return super.findRecord(store, type, id, snapshot);
}

groupRecordsForFindMany(store, snapshots) {
let result = [];
for (let i = 0; i < snapshots.length; i += BULK_REQUEST_GROUP_SIZE) {
Expand Down
8 changes: 0 additions & 8 deletions e2e/acceptance/crate-dependencies.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,6 @@ test.describe('Acceptance | crate dependencies page', { tag: '@acceptance' }, ()
server.get('/api/v1/crates/:crate_name/versions', {}, 500);
});

await ember.addHook(async owner => {
// Load `crate` and then explicitly unload the side-loaded `versions`.
let store = owner.lookup('service:store');
let crateRecord = await store.findRecord('crate', 'foo');
let versions = crateRecord.hasMany('versions').value();
versions.forEach(record => record.unloadRecord());
});

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

await expect(page).toHaveURL('/crates/foo/1.0.0/dependencies');
Expand Down
8 changes: 0 additions & 8 deletions e2e/routes/crate/range.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,14 +127,6 @@ test.describe('Route | crate.range', { tag: '@routes' }, () => {
server.get('/api/v1/crates/:crate_name/versions', {}, 500);
});

await ember.addHook(async owner => {
// Load `crate` and then explicitly unload the side-loaded `versions`.
let store = owner.lookup('service:store');
let crateRecord = await store.findRecord('crate', 'foo');
let versions = crateRecord.hasMany('versions').value();
versions.forEach(record => record.unloadRecord());
});

await page.goto('/crates/foo/range/^3');
await expect(page).toHaveURL('/crates/foo/range/%5E3');
await expect(page.locator('[data-test-404-page]')).toBeVisible();
Expand Down
6 changes: 0 additions & 6 deletions tests/acceptance/crate-dependencies-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,6 @@ module('Acceptance | crate dependencies page', function (hooks) {

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

// Load `crate` and then explicitly unload the side-loaded `versions`.
let store = this.owner.lookup('service:store');
let crateRecord = await store.findRecord('crate', 'foo');
let versions = crateRecord.hasMany('versions').value();
versions.forEach(record => record.unloadRecord());

await visit('/crates/foo/1.0.0/dependencies');
assert.strictEqual(currentURL(), '/crates/foo/1.0.0/dependencies');
assert.dom('[data-test-404-page]').exists();
Expand Down
17 changes: 17 additions & 0 deletions tests/adapters/crate-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,21 @@ module('Adapter | crate', function (hooks) {
assert.strictEqual(foo?.name, 'foo');
assert.strictEqual(bar?.name, 'bar');
});

test('findRecord requests do not include versions by default', async function (assert) {
let _foo = this.server.create('crate', { name: 'foo' });
let version = this.server.create('version', { crate: _foo });

let store = this.owner.lookup('service:store');

let foo = await store.findRecord('crate', 'foo');
assert.strictEqual(foo?.name, 'foo');

// versions should not be loaded yet
let versionsRef = foo.hasMany('versions');
assert.deepEqual(versionsRef.ids(), []);

await versionsRef.load();
assert.deepEqual(versionsRef.ids(), [version.id]);
});
});
2 changes: 1 addition & 1 deletion tests/models/version-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ module('Model | Version', function (hooks) {
this.server.create('version', { crate, num: '0.4.2' });
this.server.create('version', { crate, num: '0.4.3', yanked: true });
crateRecord = await this.store.findRecord('crate', crate.name, { reload: true });
versions = (await crateRecord.loadVersionsTask.perform()).slice();
versions = (await crateRecord.loadVersionsTask.perform({ reload: true })).slice();

assert.deepEqual(
versions.map(it => ({ num: it.num, isHighestOfReleaseTrack: it.isHighestOfReleaseTrack })),
Expand Down
6 changes: 0 additions & 6 deletions tests/routes/crate/range-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,6 @@ module('Route | crate.range', function (hooks) {

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

// Load `crate` and then explicitly unload the side-loaded `versions`.
let store = this.owner.lookup('service:store');
let crateRecord = await store.findRecord('crate', 'foo');
let versions = crateRecord.hasMany('versions').value();
versions.forEach(record => record.unloadRecord());

await visit('/crates/foo/range/^3');
assert.strictEqual(currentURL(), '/crates/foo/range/%5E3');
assert.dom('[data-test-404-page]').exists();
Expand Down

0 comments on commit 698a82c

Please sign in to comment.