Skip to content

Conversation

alan-agius4
Copy link
Collaborator

@alan-agius4 alan-agius4 commented Sep 4, 2024

This commit introduces a new server routing configuration API, as discussed in RFC angular/angular#56785. The new API provides several enhancements:

const serverRoutes: ServerRoute[] = [
  {
    path: '/error',
    renderMode: RenderMode.Server,
    status: 404,
    headers: {
      'Cache-Control': 'no-cache'
    }
  }
];
const serverRoutes: ServerRoute[] = [
  {
    path: '/product/:id',
    renderMode: RenderMode.Prerender,
    async getPrerenderParams() {
      const dataService = inject(ProductService);
      const ids = await dataService.getIds(); // Assuming this returns ['1', '2', '3']
      return ids.map(id => ({ id })); // Generates paths like: [{ id: '1' }, { id: '2' }, { id: '3' }]
    }
  }
];
const serverRoutes: ServerRoute[] = [
  {
    path: '/product/:id',
    renderMode: RenderMode.Prerender,
    fallback: PrerenderFallback.Server, // Can be Server, Client, or None
    async getPrerenderParams() {
    }
  }
];
const serverRoutes: ServerRoute[] = [
  {
    path: '/product/:id',
    renderMode: RenderMode.Server,
  },
  {
    path: '/error',
    renderMode: RenderMode.Client,
  },
  {
    path: '/**',
    renderMode: RenderMode.Prerender,
  },
];

These additions aim to provide greater flexibility and control over server-side rendering configurations and prerendering behaviors.

@angular-robot angular-robot bot added detected: feature PR contains a feature commit area: @angular/ssr labels Sep 4, 2024
@alan-agius4 alan-agius4 marked this pull request as ready for review September 4, 2024 11:39
@alan-agius4 alan-agius4 added the action: review The PR is still awaiting reviews from at least one requested reviewer label Sep 4, 2024
@alan-agius4 alan-agius4 force-pushed the render-mode branch 2 times, most recently from 6fe6e24 to 40c6fe7 Compare September 4, 2024 14:28
@alan-agius4 alan-agius4 closed this Sep 4, 2024
@angular-robot angular-robot bot removed the detected: feature PR contains a feature commit label Sep 4, 2024
@alan-agius4 alan-agius4 reopened this Sep 4, 2024
@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label Sep 4, 2024
@alan-agius4 alan-agius4 added the target: major This PR is targeted for the next major release label Sep 4, 2024
Copy link
Collaborator

@dgp1130 dgp1130 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great stuff here @alan-agius4! Glad to see this all coming together. 😁

);
}

const parameters = await runInInjectionContext(parentInjector, () => getPrerenderPaths());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: How does runInInjectionContext handle async callbacks? getPrerenderPaths() might return a Promise. What happens for:

function getPrerenderPaths() {
  await someAsyncWork();
  const service = inject(SomeService); // Can we inject after an `await`?
}

Copy link
Collaborator Author

@alan-agius4 alan-agius4 Sep 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the inject needs to happen before any async work.

function getPrerenderPaths() {
  const service = inject(SomeService); // Can we inject after an `await`?
  await someAsyncWork();
}

See: https://angular.dev/api/core/runInInjectionContext#runInInjectionContext_0

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any precedent for Angular APIs which are naturally async but run in an injection context? This feels like an easy tripping hazard.

I guess the workaround would be to keep a reference to the injector (or inject before the await like you suggested)?

async function getPrerenderPaths() {
  const injector = inject(Injector);
  const result = await someAsyncWork();
  if (result === something) {
    injector.inject(SomethingElse);
  }
}

/cc @AndrewKushnir is this something we should be concerned about?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both guards and interceptors run in an injection context.

@alan-agius4 alan-agius4 force-pushed the render-mode branch 2 times, most recently from d82eda1 to bb0ca53 Compare September 9, 2024 10:54
@alan-agius4 alan-agius4 requested a review from dgp1130 September 9, 2024 12:25
@dgp1130
Copy link
Collaborator

dgp1130 commented Sep 10, 2024

Thanks for working through this @alan-agius4! My only remaining concern is the choice of default value for PrerenderFallback, the rest I'm happy to defer to you / @AndrewKushnir. Maybe we should have a separate conversation on that point?

Copy link

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just a few minor comments, mostly related to error message text improvements.

@alan-agius4 alan-agius4 force-pushed the render-mode branch 2 times, most recently from cb69759 to f14ef08 Compare September 11, 2024 10:13
@alan-agius4 alan-agius4 force-pushed the render-mode branch 5 times, most recently from 77744f4 to df3715f Compare September 12, 2024 13:07
This commit introduces a new server routing configuration API, as discussed in RFC angular/angular#56785. The new API provides several enhancements:

```ts
const serverRoutes: ServerRoute[] = [
  {
    path: '/error',
    renderMode: RenderMode.Server,
    status: 404,
    headers: {
      'Cache-Control': 'no-cache'
    }
  }
];
```

```ts
const serverRoutes: ServerRoute[] = [
  {
    path: '/product/:id',
    renderMode: RenderMode.Prerender,
    async getPrerenderPaths() {
      const dataService = inject(ProductService);
      const ids = await dataService.getIds(); // Assuming this returns ['1', '2', '3']
      return ids.map(id => ({ id })); // Generates paths like: [{ id: '1' }, { id: '2' }, { id: '3' }]
    }
  }
];
```

```ts
const serverRoutes: ServerRoute[] = [
  {
    path: '/product/:id',
    renderMode: RenderMode.Prerender,
    fallback: PrerenderFallback.Server, // Can be Server, Client, or None
    async getPrerenderPaths() {
    }
  }
];
```

```ts
const serverRoutes: ServerRoute[] = [
  {
    path: '/product/:id',
    renderMode: RenderMode.Server,
  },
  {
    path: '/error',
    renderMode: RenderMode.Client,
  },
  {
    path: '/**',
    renderMode: RenderMode.Prerender,
  },
];
```

These additions aim to provide greater flexibility and control over server-side rendering configurations and prerendering behaviors.
@alan-agius4 alan-agius4 added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Sep 12, 2024
@alan-agius4 alan-agius4 merged commit d66aaa3 into angular:main Sep 12, 2024
31 checks passed
@alan-agius4 alan-agius4 deleted the render-mode branch September 12, 2024 17:59
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Oct 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: @angular/ssr detected: feature PR contains a feature commit target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants