-
-
Notifications
You must be signed in to change notification settings - Fork 126
Add an async context data test for @fedify/express #985
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Jae-Hyuk-Jang
wants to merge
4
commits into
fedify-dev:main
Choose a base branch
from
Jae-Hyuk-Jang:test/express-async-context-data
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+100
−2
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
64bcda0
Add an async context data test for @fedify/express
Jae-Hyuk-Jang a59ed13
Verify federation.fetch() waits for the context data promise
Jae-Hyuk-Jang 9e6c38a
Add the test:bun script for @fedify/express
Jae-Hyuk-Jang cd288b2
Drop unused response mock methods
Jae-Hyuk-Jang File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,95 @@ | ||
| import type { Request as ERequest, Response as EResponse } from "express"; | ||
| import { strict as assert } from "node:assert"; | ||
| import { describe, test } from "node:test"; | ||
| import { integrateFederation } from "./index.ts"; | ||
|
|
||
| interface MockFederation { | ||
| fetch(request: Request, options: unknown): Promise<Response>; | ||
| } | ||
|
|
||
| function createMockRequest(): ERequest { | ||
| return { | ||
| protocol: "http", | ||
| host: "localhost", | ||
| url: "/", | ||
| method: "GET", | ||
| headers: {}, | ||
| } as unknown as ERequest; | ||
| } | ||
|
|
||
| function createMockResponse(): { | ||
| response: EResponse; | ||
| ended: Promise<void>; | ||
| getBody(): string; | ||
| } { | ||
| let body = ""; | ||
| let resolveEnded: () => void; | ||
| const ended = new Promise<void>((resolve) => { | ||
| resolveEnded = resolve; | ||
| }); | ||
| const response = { | ||
| statusCode: 200, | ||
| status(code: number) { | ||
| response.statusCode = code; | ||
| return response; | ||
| }, | ||
| setHeader() { | ||
| return response; | ||
| }, | ||
| write(chunk: Buffer | string) { | ||
| body += chunk.toString(); | ||
| return true; | ||
| }, | ||
| end() { | ||
| resolveEnded(); | ||
| return response; | ||
| }, | ||
| }; | ||
| return { | ||
| response: response as unknown as EResponse, | ||
| ended, | ||
| getBody: () => body, | ||
| }; | ||
| } | ||
|
|
||
| describe("integrateFederation()", () => { | ||
| test("waits for an async contextDataFactory and passes the resolved value to federation.fetch()", async () => { | ||
| let resolveContextData!: (value: string) => void; | ||
| let fetchCalled = false; | ||
|
|
||
| const mockFederation: MockFederation = { | ||
| fetch(_request, options) { | ||
| fetchCalled = true; | ||
| const { contextData } = options as { contextData: unknown }; | ||
| return Promise.resolve(new Response(String(contextData))); | ||
| }, | ||
| }; | ||
|
|
||
| const contextDataFactory = () => | ||
| new Promise<string>((resolve) => { | ||
| resolveContextData = resolve; | ||
| }); | ||
|
|
||
| const middleware = integrateFederation( | ||
| mockFederation as never, | ||
| contextDataFactory, | ||
| ); | ||
|
|
||
| const req = createMockRequest(); | ||
| const { response, ended, getBody } = createMockResponse(); | ||
| let nextCalled = false; | ||
|
|
||
| middleware(req, response, () => { | ||
| nextCalled = true; | ||
| }); | ||
|
|
||
| await Promise.resolve(); | ||
| assert.strictEqual(fetchCalled, false); | ||
|
|
||
| resolveContextData("Hello World"); | ||
| await ended; | ||
|
|
||
| assert.strictEqual(nextCalled, false); | ||
| assert.strictEqual(getBody(), "Hello World"); | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| }); | ||
| }); | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: fedify-dev/fedify
Length of output: 486
🏁 Script executed:
Repository: fedify-dev/fedify
Length of output: 9194
🌐 Web query:
Node.js --experimental-transform-types added removed version 22.7.0 26.0.0💡 Result:
The
--experimental-transform-typesflag was added to Node.js in version 22.7.0 [1][2][3]. It was subsequently removed in version 26.0.0 [1][4]. The removal was implemented via pull request#61803, which was merged on February 27, 2026 [5][6].Citations:
🏁 Script executed:
Repository: fedify-dev/fedify
Length of output: 385
🌐 Web query:
fedify-dev fedify package.json packageManager package.json engines node pnpm-workspace.yaml fedify💡 Result:
Fedify is a TypeScript-based framework designed for building federated server applications, supporting multiple runtimes (Node.js, Deno, Bun) and various package managers [1][2][3]. The technical configuration details regarding the requested files are as follows: pnpm-workspace.yaml The Fedify repository is a monorepo that utilizes pnpm workspaces to manage its various packages (e.g., core, web framework integrations, CLI) [1][4][5]. The pnpm-workspace.yaml file defines the packages included in the workspace and employs the pnpm catalog feature to manage dependencies centrally [4][6]. When adding a new package or updating existing ones, dependencies in individual package.json files reference these catalog versions using the catalog: protocol rather than hardcoded version numbers [6]. package.json and packageManager Fedify's CLI includes a fedify init command that scaffolds new projects [2]. This command allows users to specify a package manager (options include deno, pnpm, bun, yarn, or npm) [7][8]. The scaffolder generates a package.json file for Node.js-based projects, which includes essential metadata such as "type": "module" and project-specific npm scripts [9]. While the CLI dynamically manages dependency installation based on the chosen runtime and package manager, it does not mandate a specific packageManager field in the generated package.json [10][9]. Historically, issues regarding the CLI's detection of local package managers (e.g., pnpm) have been addressed in maintenance updates [11]. engines node Fedify-based projects are generally compatible with standard Node.js environments. For example, specific implementations, such as the federated microblog example, require Node.js 20.0.0 or later [3]. While developers may define an engines field in their project-level package.json to enforce Node.js version compatibility, this is standard practice for Node.js projects rather than a specific requirement enforced by the Fedify framework itself. Overall, Fedify is designed to be runtime-agnostic and modular, allowing users to integrate it into their preferred Node.js frameworks (e.g., Express, Fastify, Next.js) using the appropriate integration package [12][2].
Citations:
Scope the Node command to a supported runtime.
This package does not declare Node version support, while
--experimental-transform-typesexists only for Node 22.7.0 through 25. Addengines.nodefor the supported range or use a Node configuration that is compatible with the declared support baseline.🤖 Prompt for AI Agents