[ENG-9792] Angular Universal#803
[ENG-9792] Angular Universal#803brianjgeiger merged 14 commits intoCenterForOpenScience:feature/pbs-25-24from
Conversation
brianjgeiger
left a comment
brianjgeiger
left a comment
There was a problem hiding this comment.
Overall, looks good. I had a couple of questions on some removed code.
Is it feasible to add tests to ensure that the pages that we want to render server-side will be renderable server-side? I'd hate for us to add something that breaks the functionality without realizing it.
And my understanding is that this should have no effect on client-side rendering, it would only take effect when running a server instance and we push people to that server instance. Is that correct? So we'd be safe to merge this with a full regression test from QA to make sure we didn't accidentally break anything?
| const currentResource = store.selectSnapshot(CurrentResourceSelectors.getCurrentResource); | ||
| const currentUser = store.selectSnapshot(UserSelectors.getCurrentUser); | ||
|
|
||
| if (currentResource && !id.startsWith(currentResource.id)) { | ||
| if (currentResource.type === CurrentResourceType.Projects && currentResource.parentId) { | ||
| router.navigate(['/', currentResource.parentId, 'files', id], { queryParamsHandling: 'preserve' }); | ||
| return true; | ||
| } | ||
|
|
||
| if (currentResource.type === CurrentResourceType.Preprints && currentResource.parentId) { | ||
| router.navigate(['/preprints', currentResource.parentId, id]); | ||
| return true; | ||
| } | ||
|
|
||
| if (currentResource.type === CurrentResourceType.Users) { | ||
| if (currentUser && currentUser.id === currentResource.id) { | ||
| router.navigate(['/profile']); | ||
| } else { | ||
| router.navigate(['/user', id]); | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| return currentResource.type === CurrentResourceType.Projects; | ||
| } |
There was a problem hiding this comment.
Is this not important? I didn't see a replacement for it anywhere, but maybe we're not really using it?
There was a problem hiding this comment.
I removed duplication. We already have same logic below. The GetResource action handles caching (it won't refetch if the same resource is already loaded).
| const currentResource = store.selectSnapshot(CurrentResourceSelectors.getCurrentResource); | ||
| const currentUser = store.selectSnapshot(UserSelectors.getCurrentUser); | ||
|
|
||
| if (currentResource && !id.startsWith(currentResource.id)) { | ||
| if (currentResource.type === CurrentResourceType.Registrations && currentResource.parentId) { | ||
| router.navigate(['/', currentResource.parentId, 'files', id], { queryParamsHandling: 'preserve' }); | ||
| return true; | ||
| } | ||
|
|
||
| if (currentResource.type === CurrentResourceType.Preprints && currentResource.parentId) { | ||
| router.navigate(['/preprints', currentResource.parentId, id]); | ||
| return true; | ||
| } | ||
|
|
||
| if (currentResource.type === CurrentResourceType.Users) { | ||
| if (currentUser && currentUser.id === currentResource.id) { | ||
| router.navigate(['/profile']); | ||
| } else { | ||
| router.navigate(['/user', id]); | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| return currentResource.type === CurrentResourceType.Registrations; | ||
| } |
Yeah, I can add unit tests for pages with SSR. Issues can occur only during build time, initial loading, or hydration (when the browser receives the SSR HTML). After that, it will run using CSR. |
Yes, that's correct. It has no effect on client-side rendering. After the browser receives the SSR HTML and hydrates, all subsequent navigation is standard client-side SPA behavior, just like a non-SSR Angular app. |
brianjgeiger
merged commit c75c518
into
CenterForOpenScience:feature/pbs-25-24
- Ticket: [ENG-9792] - Feature flag: n/a ## Summary of Changes 1. Added SSR.
2 participants
Summary of Changes