fix: after-middleware re-entry, notification read state, and DotPath/type/build fixes

CHANGELOG.md

@@ -24,6 +24,11 @@ The format follows semantic versioning principles.
 
 ### Fixed
 
+- `DotPath` (the key type behind `config()` and `configure()`) is now depth-bounded and stops at built-in object types, so configs holding deep or self-referencing library types (nodemailer transports) no longer fail to typecheck with "circularly references itself". The `@ts-expect-error` workarounds in the H3 template and the notifications config are gone.
+- `build:templates` builds the templates one at a time; run in parallel, each template's prebuild rebuilt the shared packages concurrently and the builds clobbered each other.
+- Fixed type errors in `@arkstack/auth` (`AuthenticationException` request typing and two tests) and in `@arkstack/common`'s `renderError`, which used the `view()` global without a type for it.
+- Express `after` middleware no longer resumes the router once the response has finished. It used to receive the real `next`, so a middleware such as `requestLogger()` sent each request on through the remaining routes after responding, raising "Cannot set headers after they are sent to the client".
+- `UserNotificationCenter.markAllRead()` now marks every unread notification read; it used a single-row `update()` and marked only one. `unreadForUser(user, perPage)` now honours `perPage`, and both filter unread rows with `whereNull('readAt')`.
 - `UserNotificationCenter.send()` (and `Notification.db().broadcast()`) now broadcast on the user's channel. They previously sent to the user's push tokens as channels, so nothing was broadcast at all for users without tokens. Apps on the `firebase` transport still reach their users' devices without changes, because device push is on there unless `drivers.realtime.push` is `false`.
 - `Notification.db().broadcast()` now returns the builder, so it chains as documented.
 - The Pusher auth route registered by `registerAuthRoute()` now answers with HTTP 200 (pusher-js rejects any other status), registers on the H3 driver instead of the Express one, and authorizes through the channel registry.

package.json

@@ -13,7 +13,7 @@
     "test:watch": "vitest",
     "test:coverage": "vitest run --coverage",
     "test:integration": "vitest run tests/integration",
-    "build:templates": "pnpm -r --filter \"*-app\" build",
+    "build:templates": "pnpm -r --workspace-concurrency=1 --filter \"*-app\" build",
     "build:packages": "pnpm -r --filter \"@arkstack/*\" build",
     "prepare:release-version": "node scripts/prepare-release-version.mjs",
     "publish:packages": "pnpm -r --filter \"@arkstack/*\" --filter create-arkstack publish --access public",

packages/auth/src/Exceptions/AuthenticationException.ts

@@ -1,18 +1,19 @@
 import { Request, Response, type RequestSource, type ResponseSource } from '@arkstack/http'
 
 import { Exception } from '@arkstack/common'
+import type { User } from '@app/models/User'
 
 export class AuthenticationException extends Exception {
     #errors?: Record<string, any>
-    #request?: Request
+    #request?: Request<User>
     #response?: Response
     statusCode: number = 401
     name: string
 
     constructor(
         message: string = 'Authentication failed',
         ctx?: {
-            req?: Request | RequestSource,
+            req?: Request<User> | RequestSource<User>,
             res?: Response | ResponseSource,
             status?: number,
             errors?: Record<string, any>
@@ -31,7 +32,7 @@ export class AuthenticationException extends Exception {
         void this.#request
     }
 
-    errors (): Record<string, any> | undefined {
+    errors(): Record<string, any> | undefined {
         return this.#errors
     }
 }

packages/auth/tests/express.test.ts

@@ -79,12 +79,12 @@ describe('Express auth integration', () => {
             return {
                 authToken: req?.authToken,
                 authUserFromAuthId: (req?.auth as Auth | undefined)?.user()?.id,
-                authUserId: req?.authUser?.id,
+                authUserId: (req?.authUser as { id?: unknown } | undefined)?.id,
                 constructorName: req?.constructor.name,
                 hasIp: Boolean(req?.ip),
                 hasSource: Boolean(req?.source),
                 isArkstackRequest: req instanceof ArkstackRequest,
-                userId: req?.user?.id,
+                userId: (req?.user as { id?: unknown } | undefined)?.id,
             }
         }, [auth])
         Router.apply(router)

packages/auth/tests/two-factor.test.ts

@@ -8,7 +8,7 @@ const user = {
     email: 'two-factor@example.com',
     name: 'Two Factor',
     password: 'password',
-} as User
+} as unknown as User
 
 describe('TwoFactor', () => {
     it('normalizes supported methods', () => {

packages/common/package.json

@@ -64,5 +64,10 @@
         "arkormx": {
             "optional": true
         }
+    },
+    "inlinedDependencies": {
+        "clear-router": "2.9.3",
+        "dayjs": "1.11.20",
+        "kanun": "1.2.0"
     }
 }

packages/common/src/network.ts

@@ -45,6 +45,11 @@ export const bootWithDetectedPort = async <TApp, TRoutes = unknown, THandler = u
   if (Hook.has('boot', 'after')) Hook.get('boot', 'after', port, app)
 }
 
+/** The `view()` global registered by `@arkstack/view`. */
+type ViewGlobal = {
+  view: (name: string, data?: Record<string, unknown>) => { renderSync(): string }
+}
+
 export const renderError = ({
   message = 'An unexpected error occurred.',
   stack,
@@ -69,7 +74,7 @@ export const renderError = ({
 
   title = titleMap[code] || title || 'Error'
 
-  return globalThis.view('~arkstack/common.error', {
+  return (globalThis as unknown as ViewGlobal).view('~arkstack/common.error', {
     code,
     title,
     stack,

packages/common/src/types.ts

@@ -140,16 +140,29 @@ export type DotPathValue<T, P extends string> =
     ? T[P]
     : never
 
-export type DotPath<T> = T extends Primitive
+/** Remaining depth for {@link DotPath}: index by the current depth to get the next. */
+type DotPathDepth = [never, 0, 1, 2, 3, 4, 5, 6]
+
+/** Values {@link DotPath} treats as leaves instead of walking into. */
+type DotPathLeaf = Primitive | Date | RegExp | Promise<unknown> | Map<unknown, unknown> | Set<unknown> | Uint8Array
+
+/**
+ * Dot-separated key paths of `T`, e.g. `'mail.from.address'`. Bounded in depth
+ * so deep or self-referencing library types (mail transports, streams) stay
+ * finite instead of being reported as circular.
+ */
+export type DotPath<T, Depth extends number = 7> = [Depth] extends [never]
+    ? never
+    : T extends DotPathLeaf
     ? never
     : T extends any[]
     ? never
     : {
-        [K in keyof T & string]: T[K] extends Primitive
+        [K in keyof T & string]: T[K] extends DotPathLeaf
         ? `${K}`
         : T[K] extends any[]
         ? `${K}`
-        : `${K}` | `${K}.${DotPath<T[K]>}`
+        : `${K}` | `${K}.${DotPath<T[K], DotPathDepth[Depth]>}`
     }[keyof T & string]
 
 /**

packages/driver-express/src/ExpressDriver.ts

@@ -120,9 +120,12 @@ export class ExpressDriver extends ArkstackKitDriver<Express, Handler> {
                 const entry = resolveMiddleware(instance)
 
                 if (pos === 'after') {
-                    app.use(async (req, res, next) => {
-                        res.once('finish', async () => {
-                            await entry(req, res, next)
+                    app.use((req, res, next) => {
+                        // The response is already sent, so `next` must not resume the router.
+                        res.once('finish', () => {
+                            Promise.resolve(entry(req, res, () => { })).catch((error) => {
+                                Logger.error(error instanceof Error ? error.message : String(error))
+                            })
                         })
                         next()
                     })

packages/driver-express/tests/after-middleware.test.ts

@@ -0,0 +1,33 @@
+import { describe, expect, it, vi } from 'vitest'
+import express from 'express'
+import request from 'parasito'
+
+import { ExpressDriver } from '../src/index'
+
+describe('ExpressDriver after middleware', () => {
+    it('runs once the response finishes without re-entering the router', async () => {
+        const app = express()
+        const handler = vi.fn((_req: express.Request, res: express.Response) => {
+            res.json({ ok: true })
+        })
+        const fallthrough = vi.fn((_req: express.Request, _res: express.Response, next: express.NextFunction) => next())
+        let finished!: () => void
+        const done = new Promise<void>((resolve) => (finished = resolve))
+        const after = vi.fn((_req: unknown, _res: unknown, next: () => void) => {
+            next()
+            finished()
+        })
+
+        new ExpressDriver({ bindRouter: vi.fn() }).applyMiddleware(app, { after: [after] } as never)
+        app.get('/thing', handler)
+        app.use(fallthrough)
+
+        const res = await request(app).get('/thing')
+        await done
+
+        expect(res.status).toBe(200)
+        expect(after).toHaveBeenCalledOnce()
+        expect(handler).toHaveBeenCalledOnce()
+        expect(fallthrough).not.toHaveBeenCalled()
+    })
+})

packages/driver-h3/tests/after-middleware.test.ts

@@ -0,0 +1,22 @@
+import { describe, expect, it, vi } from 'vitest'
+import { H3 } from 'h3'
+
+import { H3Driver } from '../src/index'
+
+describe('H3Driver after middleware', () => {
+    it('runs after the handler, which runs once, and keeps its response', async () => {
+        const app = new H3()
+        const handler = vi.fn(() => ({ ok: true }))
+        const after = vi.fn((_event: unknown, next: () => unknown) => next())
+
+        new H3Driver({ bindRouter: vi.fn() } as never).applyMiddleware(app, { after: [after] } as never)
+        app.get('/thing', handler)
+
+        const res = await app.fetch(new Request('http://localhost/thing'))
+
+        expect(res.status).toBe(200)
+        expect(await res.json()).toEqual({ ok: true })
+        expect(handler).toHaveBeenCalledOnce()
+        expect(after).toHaveBeenCalledOnce()
+    })
+})

packages/http/src/Request.ts

@@ -48,35 +48,35 @@ export class Request<TUser = User> extends BaseRequest {
     private currentAuthUser?: TUser
     private currentAuthToken?: string
 
-    get user (): TUser | undefined {
+    get user(): TUser | undefined {
         return this.getSourceRequest()?.user ?? this.currentUser
     }
 
-    set user (user: TUser | undefined) {
+    set user(user: TUser | undefined) {
         this.currentUser = user
     }
 
-    get auth (): unknown {
+    get auth(): unknown {
         return this.getSourceRequest()?.auth ?? this.currentAuth
     }
 
-    set auth (auth: unknown) {
+    set auth(auth: unknown) {
         this.currentAuth = auth
     }
 
-    get authUser (): TUser | undefined {
+    get authUser(): TUser | undefined {
         return this.getSourceRequest()?.authUser ?? this.currentAuthUser
     }
 
-    set authUser (user: TUser | undefined) {
+    set authUser(user: TUser | undefined) {
         this.currentAuthUser = user
     }
 
-    get authToken (): string | undefined {
+    get authToken(): string | undefined {
         return this.getSourceRequest()?.authToken ?? this.currentAuthToken
     }
 
-    set authToken (token: string | undefined) {
+    set authToken(token: string | undefined) {
         this.currentAuthToken = token
     }
 
@@ -105,7 +105,7 @@ export class Request<TUser = User> extends BaseRequest {
         setFallbackRequest(this)
     }
 
-    static from<TUser = unknown> (
+    static from<TUser = User>(
         source?: Request<TUser> | RequestSource<TUser>
     ): Request<TUser> | undefined {
         if (!source) {
@@ -139,11 +139,11 @@ export class Request<TUser = User> extends BaseRequest {
         })
     }
 
-    header (name: string): string {
+    header(name: string): string {
         return this.headers[name.toLowerCase()]
     }
 
-    bearerToken (): string | null {
+    bearerToken(): string | null {
         const authorization = this.header('authorization')
 
         if (!authorization?.startsWith('Bearer ')) {
@@ -153,7 +153,7 @@ export class Request<TUser = User> extends BaseRequest {
         return authorization.substring(7)
     }
 
-    setUser (user: TUser) {
+    setUser(user: TUser) {
         this.user = user
 
         if (isRecord(this.source)) {
@@ -163,7 +163,7 @@ export class Request<TUser = User> extends BaseRequest {
         return this
     }
 
-    setAuthentication<TAuth> (auth: TAuth, user: TUser, token?: string) {
+    setAuthentication<TAuth>(auth: TAuth, user: TUser, token?: string) {
         this.auth = auth
         this.authUser = user
         this.authToken = token
@@ -178,7 +178,7 @@ export class Request<TUser = User> extends BaseRequest {
         return this
     }
 
-    syncFromSource () {
+    syncFromSource() {
         if (!isRecord(this.source)) {
             return this
         }
@@ -193,13 +193,13 @@ export class Request<TUser = User> extends BaseRequest {
         return this
     }
 
-    private getSourceRequest (): RequestSource<TUser> | undefined {
+    private getSourceRequest(): RequestSource<TUser> | undefined {
         return isRecord(this.source)
             ? this.source as RequestSource<TUser>
             : undefined
     }
 
-    clearAuthentication () {
+    clearAuthentication() {
         this.auth = undefined
         this.authUser = undefined
         this.authToken = undefined

packages/notifications/src/UserNotificationCenter.ts

@@ -78,10 +78,10 @@ export class UserNotificationCenter {
     static async unreadForUser(user: User, perPage?: number): Promise<unknown> {
         const Model = await this.getModel()
 
-        const query = Model.query().where({ userId: user.id, readAt: null })
+        const query = Model.query().where({ userId: user.id }).whereNull('readAt')
 
         if (perPage) {
-            return await query.paginate()
+            return await query.paginate(perPage)
         }
 
         return await query.get()
@@ -112,13 +112,11 @@ export class UserNotificationCenter {
     static async markAllRead(user: User, ids?: string[] | number[]) {
         const Model = await this.getModel()
 
-        const query = Model.query().where({ userId: user.id, readAt: null })
+        const query = Model.query().where({ userId: user.id }).whereNull('readAt')
+        const scoped = ids ? query.whereIn('id', ids) : query
 
-        if (ids) {
-            query.whereIn('id', ids)
-        }
-
-        await query.update({ readAt: new Date() })
+        // `update` changes a single row; `updateFrom` updates every match.
+        await scoped.updateFrom({ readAt: new Date() })
     }
 
     /**

packages/notifications/src/config.ts

@@ -2,7 +2,6 @@ import { DotPath, DotPathValue, config } from '@arkstack/common'
 
 import { NotificationConfig } from './types'
 
-// @ts-expect-error Just ignore the old fool
 export const configure = <T extends DotPath<NotificationConfig>>(
     key: T,
     defaultValue?: unknown,