Conversation
axelhamil
left a comment
There was a problem hiding this comment.
Thanks for the contribution! Great first PR, the JSDoc additions are solid and really improve the IntelliSense experience. The distinction between scope() and extend(), as well as the ContainerWarning.type descriptions, are particularly helpful.
Two things need to be fixed before merging, plus a small request regarding the commit message:
The reset() doc is factually incorrect: see inline comment.
The wording for extend() is confusing: see inline comment.
Commit message: the project uses Conventional Commits for semantic-release. Could you amend it to:
docs: add JSDoc comments to public type definitions?
Once those are addressed, it’s good to go 👍
| * Does not affect parent scopes. | ||
| * Does not affect parent containers in a scope chain. | ||
| * | ||
| * @param keys - Keys to reset. If none provided, this is a no-op. |
There was a problem hiding this comment.
This is incorrect, reset() with no arguments clears the entire cache, init state, dep graph, and warnings. It's a full reset, not a no-op.
Go to container-proxy.ts file l.78
| * @param keys - Keys to reset. If none provided, this is a no-op. | |
| @param keys - Keys to reset. If none provided, resets ALL cached singletons and clears all internal state. |
| * Returns a new container with additional dependencies. | ||
| * Existing singletons are shared. The original container is not modified. | ||
| * Returns a new container with additional dependencies merging with the current ones. | ||
| * Unlike `scope()`, the existing singleton cache is SHARED (copied). |
There was a problem hiding this comment.
The wording "SHARED (copied)" is a bit confusing, it reads as contradictory. The internal code uses "snapshot-copied" which conveys the idea more clearly: it's a point-in-time copy of the cache, after which each container lives independently.
| * Unlike `scope()`, the existing singleton cache is SHARED (copied). | |
| * Unlike `scope()`, already-resolved singletons are snapshot-copied into the new container. |
Summary of Changes:
Comprehensive API Documentation: Added JSDoc to all methods in IContainer (e.g., scope, extend, preload, dispose) including parameter descriptions and usage examples.
Type Metadata: Documented all exported types and interfaces, including Container, ContainerGraph, ProviderInfo, and ContainerHealth.
IntelliSense Enhancements: Properties and options like ScopeOptions.name now include descriptive comments.
Project Alignment: Comments are concise and aligned with the 100 character line width and existing Biome styling.
Closes issue #4