Merge pull request #10296 from eth3lbert/app-crate-without-versions

app: Make separate requests for crate and versions

Author: Turbo87

app/adapters/crate.js

@@ -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) {

app/models/crate.js

@@ -130,8 +130,10 @@ export default class Crate extends Model {
     return [...(teams ?? []), ...(users ?? [])];
   });
 
-  loadVersionsTask = task(async () => {
-    return (await this.versions) ?? [];
+  loadVersionsTask = task(async ({ reload = false } = {}) => {
+    let versionsRef = this.hasMany('versions');
+    let fut = reload === true ? versionsRef.reload() : versionsRef.load();
+    return (await fut) ?? [];
   });
 }
 

app/routes/crate/dependencies.js

@@ -6,7 +6,7 @@ export default class VersionRoute extends Route {
 
   async model() {
     let crate = this.modelFor('crate');
-    let versions = await crate.get('versions');
+    let versions = await crate.loadVersionsTask.perform();
 
     let { default_version } = crate;
     let version = versions.find(version => version.num === default_version) ?? versions.lastObject;

app/routes/crate/range.js

@@ -15,7 +15,7 @@ export default class VersionRoute extends Route {
     let crate = this.modelFor('crate');
 
     try {
-      let versions = await crate.hasMany('versions').load();
+      let versions = await crate.loadVersionsTask.perform();
       let allVersionNums = versions.map(it => it.num);
       let unyankedVersionNums = versions.filter(it => !it.yanked).map(it => it.num);
 

app/routes/crate/version-dependencies.js

@@ -9,7 +9,7 @@ export default class VersionRoute extends Route {
 
     let versions;
     try {
-      versions = await crate.get('versions');
+      versions = await crate.loadVersionsTask.perform();
     } catch (error) {
       let title = `${crate.name}: Failed to load version data`;
       return this.router.replaceWith('catch-all', { transition, error, title, tryAgain: true });

app/routes/crate/version.js

@@ -16,7 +16,7 @@ export default class VersionRoute extends Route {
 
     let versions;
     try {
-      versions = await crate.get('versions');
+      versions = await crate.loadVersionsTask.perform();
     } catch (error) {
       let title = `${crate.name}: Failed to load version data`;
       return this.router.replaceWith('catch-all', { transition, error, title, tryAgain: true });

e2e/acceptance/crate-dependencies.spec.ts

@@ -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');

e2e/routes/crate/range.spec.ts

@@ -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();

tests/acceptance/crate-dependencies-test.js

@@ -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();

tests/adapters/crate-test.js

@@ -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]);
+  });
 });