Skip to content

Conversation

skonves
Copy link
Owner

@skonves skonves commented Apr 19, 2025

Resolves #78
Resolves #79

@skonves skonves requested a review from Copilot April 19, 2025 22:34
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR moves the AsyncLocalStorage instance from module scope to global scope and adds an integration test to verify that context values remain unique across different usage paths.

  • Moves AsyncLocalStorage instance initialization to globalThis in index.js.
  • Introduces an init function and supporting constants in the intermediate library.
  • Adds a new integration test to ensure unique context propagation.

Reviewed Changes

Copilot reviewed 7 out of 9 changed files in this pull request and generated no comments.

File Description
tests/express-test-harness.js Adds integration test to verify unique context values when the library is used both directly and transitively.
packages/express-http-context-intermediate-library/index.js Introduces an init function and constants for managing request IDs via middleware.
packages/express-http-context-intermediate-library/README.md Adds instructions for publishing the library to NPM.
index.js Shifts AsyncLocalStorage instance creation to a global scope using globalThis.
Files not reviewed (2)
  • package.json: Language not supported
  • packages/express-http-context-intermediate-library/package.json: Language not supported
Comments suppressed due to low confidence (2)

index.js:13

  • [nitpick] Consider caching the global instance in the middleware, get, and set functions consistently to improve readability and avoid repeated global lookups.
const asyncLocalStorage = globalThis[STORAGE_KEY];

packages/express-http-context-intermediate-library/index.js:6

  • [nitpick] Ensure that the naming for the request ID header constants is consistent and clearly distinguishes between the incoming header and the header set in the response, if appropriate.
const REQUEST_ID_HTTP_HEADER_NAME = "my-request-id";

@coveralls
Copy link

Coverage Status

coverage: 100.0%. remained the same
when pulling 46ad2c3 on issue-78
into d8c1e4c on master.

@skonves skonves merged commit 15d6c37 into master Apr 19, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Library does not support interactions with the same context from both a library and a consuming application
2 participants