app: Make separate requests for crate and versions #10296
Conversation
| 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; |
There was a problem hiding this comment.
doesn't have to be in this PR, but this should probably just this.router.replaceWith('crate.version-dependencies', crate, default_version); instead of loading the full version list 😅
There was a problem hiding this comment.
Yeah, the part after #10288 needs to be changed. I'm also wondering if we could just load versions in the versions route. (This would have the downside of not having a versions count on the tab.)
There was a problem hiding this comment.
I'm also wondering if we could just load versions in the versions route. (This would have the downside of not having a versions count on the tab.)
I forgot that we show the number of versions in the tab. I do look at that quite often though, so it would be unfortunate to lose it 🤔
| versions = await crate.get('versions'); | ||
| versions = await crate.loadVersionsTask.perform(); |
There was a problem hiding this comment.
again, not in this PR: but this should probably also only request the corresponding version instead of the full version list :D
| versions = await crate.get('versions'); | ||
| versions = await crate.loadVersionsTask.perform(); |
There was a problem hiding this comment.
same as above, this shouldn't need to load the full version list to display a single version
| }); | ||
|
|
||
| await page.goto('/crates/foo/1.0.0/dependencies'); | ||
|
|
||
| await expect(page).toHaveURL('/crates/foo/1.0.0/dependencies'); | ||
| await page.waitForFunction(() => globalThis.version_ids?.length === 0); |
There was a problem hiding this comment.
what is the reasoning behind this assertion? at first glance I think I'd just remove the whole ember.addHook() call above since it's no longer needed 🤔
There was a problem hiding this comment.
I was trying to do the same assertion as done in QUnit. But yeah, I think we could just remove the assertion for both.
eth3lbert
force-pushed
the
app-crate-without-versions
branch
from
1ac2d14 to
bdf7ddd
Compare
… 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.
eth3lbert
force-pushed
the
app-crate-without-versions
branch
from
bdf7ddd to
698a82c
Compare
Extracted from #10248.
This will ensure that the
versionsare fetched via a separate requests, rather than being included in thecrate'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.Before:
full-versions-included.mov
After:
versions-not-included.mov