fix(@angular/ssr): allow using function with optional request context argument

[pieh]

PR Checklist

Please check to confirm your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/angular/angular-cli/blob/main/CONTRIBUTING.md#-commit-message-guidelines
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

• Bugfix
• Feature
• Code style update (formatting, local variables)
• Refactoring (no functional changes, no api changes)
• Build related changes
• CI related changes
• Documentation content changes
• Other... Please describe:

What is the current behavior?

When using createRequestHandler on Netlify edge function compatible request handler function I am forced to use @ts-expect-error directive currently:

import { AngularAppEngine, createRequestHandler } from '@angular/ssr'
const angularAppEngine = new AngularAppEngine()

// @ts-expect-error - createRequestHandler expects a function with single Request argument and doesn't allow context argument
export const reqHandler = createRequestHandler(async (request: Request, context: any) => {
  const result = await angularAppEngine.handle(request, context)
  return result || new Response('Not found', { status: 404 })
})

Issue Number: N/A

What is the new behavior?

Don't need to use @ts-expect-error directive anymore.

import { AngularAppEngine, createRequestHandler } from '@angular/ssr'
const angularAppEngine = new AngularAppEngine()

export const reqHandler = createRequestHandler(async (request: Request, context: any) => {
  const result = await angularAppEngine.handle(request, context)
  return result || new Response('Not found', { status: 404 })
})

Does this PR introduce a breaking change?

• Yes
• No

Other information

From what I understand applying metadata done by createRequestHandler is used to recognize in dev mode that exported handle can be used to handle requests and falling back to "default" when it's not (and ignoring any customizations in server.ts file in dev mode because of it), so there doesn't seem to exist workaround the current type while still having access to second parameter?

[alan-agius4]

Instead of using any here, can we instead use generics to infer the type?

Example

export type RequestHandlerFunction<T = unknown> = (
  request: Request,
  requestContext?: T,
) => Promise<Response | null> | null | Response;

export function createRequestHandler<T = unknown>(
  handler: RequestHandlerFunction<T>,
): RequestHandlerFunction<T> {

}

[pieh]

I found alternative that doesn't require type modification - if anyone is interested it's something like this (attempted to simplify it in case other would face similar issue to solve):

import { AsyncLocalStorage } from 'node:async_hooks'

import { AngularAppEngine, createRequestHandler } from '@angular/ssr'
import type { Context } from '@netlify/edge-functions'

const angularAppEngine = new AngularAppEngine()
const ContextStorage = new AsyncLocalStorage<Context>()

function requestHandlerImpl(request: Request) {
  const context = ContextStorage.getStore()
  // context will be undefined in dev, but should be defined in prod

  return angularAppEngine.handle(request, context)
}

// this will be production deployment entry point - it sets context on the async local storage and we can access it later in common handler implementation without passing it as function argument
export default function prodHandler(request: Request, context: Context) {
  return ContextStorage.run(context, () => requestHandlerImpl(request))
}

// dev
export const reqHandler = createRequestHandler(requestHandlerImpl)

So I'll close this PR as given that there is pretty portable solution that can be adjusted to work with any provider - attempting to adjust the types for specific signature is unnecessary risk to take and would be annoying to deal with in the future if another incompatible signature would need to be handled.

[angular-automatic-lock-bot]

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.}