Skip to content

updated types.ts#18

Open
Chidwan3578 wants to merge 1 commit intoaxelhamil:mainfrom
Chidwan3578:main
Open

updated types.ts#18
Chidwan3578 wants to merge 1 commit intoaxelhamil:mainfrom
Chidwan3578:main

Conversation

@Chidwan3578
Copy link

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

Copy link
Owner

@axelhamil axelhamil left a comment

Choose a reason for hiding this comment

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

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.
Copy link
Owner

Choose a reason for hiding this comment

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

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

Suggested change
* @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).
Copy link
Owner

Choose a reason for hiding this comment

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

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.

Suggested change
* Unlike `scope()`, the existing singleton cache is SHARED (copied).
* Unlike `scope()`, already-resolved singletons are snapshot-copied into the new container.

@axelhamil axelhamil added documentation Improvements or additions to documentation dx Developer experience labels Feb 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation dx Developer experience

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants