Skip to content

Conversation

@sciborrudnicki
Copy link
Contributor

No description provided.

@sciborrudnicki sciborrudnicki merged commit b618e3f into main Dec 7, 2025
4 checks passed
Copy link

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 introduces the initial release (v0.0.1) of @typescript-package/collection, a lightweight TypeScript library for data collection management with adapter pattern support.

Key Changes:

  • Implements core collection abstractions (CollectionBase, CollectionCore) with generic type support
  • Adds SetAdapter implementation for Set-based collections and concrete Collection class
  • Includes comprehensive documentation, test setup with Vitest, and Angular build configuration

Reviewed changes

Copilot reviewed 18 out of 20 changed files in this pull request and generated 19 comments.

Show a summary per file
File Description
package.json Defines package metadata, peer dependencies, and publishing configuration
package-lock.json Locks dependency versions for @typedly/collection and transitive dependencies
README.md Provides comprehensive documentation with installation instructions, API reference, and contribution guidelines
tsconfig.*.json Configures TypeScript compilation for library and test builds
ng-package.json Configures Angular package build settings
src/public-api.ts Exports public API surface with SetAdapter, core classes, and Collection
src/core/lib/collection.core.ts Defines abstract core class with collection shape interface
src/core/lib/collection.base.ts Implements base collection functionality with adapter support
src/adapter/lib/set.adapter.ts Implements Set-based collection adapter
src/lib/collection.class.ts Provides concrete Collection class with generic adapter support
src/test/collection.spec.ts Contains basic test cases for Collection with SetAdapter
.gitignore Specifies files and directories to exclude from version control
.github/FUNDING.yml Configures project funding and sponsorship options

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

console.log(`size: `, collection.size); // Output: 5
```

[`Collection`](https://github.com/typescript-package/hooks/blob/main/src/lib/collection.ts)
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

Incorrect GitHub repository link and wrong file name. The URL points to typescript-package/hooks but should point to typescript-package/collection. Additionally, the file is named collection.class.ts, not collection.ts.

Copilot uses AI. Check for mistakes.
<!-- This package: typescript-package -->
<!-- GitHub: badges -->
[typescript-package-badge-issues]: https://img.shields.io/github/issues/typescript-package/collection
[isscript-package-badge-forks]: https://img.shields.io/github/forks/typescript-package/collection
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

Typo in badge reference name: isscript-package-badge-forks should be typescript-package-badge-forks.

Suggested change
[isscript-package-badge-forks]: https://img.shields.io/github/forks/typescript-package/collection
[typescript-package-badge-forks]: https://img.shields.io/github/forks/typescript-package/collection

Copilot uses AI. Check for mistakes.
// Deletes.
collection.delete(29, 31);

console.log(`size: `, collection.size); // Output: 5
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

This console.log statement should be removed or moved inside a test case. Test files should not contain debug logging statements in the module scope, as they produce output during test execution.

Suggested change
console.log(`size: `, collection.size); // Output: 5

Copilot uses AI. Check for mistakes.
Comment on lines +57 to +83
return element.forEach(e => this.#adapter.add(e)), this;
}
public clear(): this {
return this.#adapter.clear(), this;
}
public clone(): CollectionCore<E, T> {
return new (this.constructor as any)(this.#adapter.value, this.#adapter);
}
public destroy(): this {
return this.clear(),
this.#adapter = null as any,
this;
}
public delete(...element: E[]): boolean {
return element.every(e => this.#adapter.delete(e));
}
public forEach(callbackfn: (element: E, element2: E, collection: CollectionCore<E, T>) => void, thisArg?: any): this {
return this.#adapter.forEach(callbackfn as any, thisArg), this;
}
public has(element: E): boolean {
return this.#adapter.has(element);
}
public lock(): this {
return this.#adapter.lock?.(), this;
}
public set(value: T): this {
return this.#adapter.set(value), this;
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

The comma operator usage makes this code harder to read and understand. Consider refactoring to separate statements for better clarity:

public clear(): this {
  this.#adapter.clear();
  return this;
}
Suggested change
return element.forEach(e => this.#adapter.add(e)), this;
}
public clear(): this {
return this.#adapter.clear(), this;
}
public clone(): CollectionCore<E, T> {
return new (this.constructor as any)(this.#adapter.value, this.#adapter);
}
public destroy(): this {
return this.clear(),
this.#adapter = null as any,
this;
}
public delete(...element: E[]): boolean {
return element.every(e => this.#adapter.delete(e));
}
public forEach(callbackfn: (element: E, element2: E, collection: CollectionCore<E, T>) => void, thisArg?: any): this {
return this.#adapter.forEach(callbackfn as any, thisArg), this;
}
public has(element: E): boolean {
return this.#adapter.has(element);
}
public lock(): this {
return this.#adapter.lock?.(), this;
}
public set(value: T): this {
return this.#adapter.set(value), this;
element.forEach(e => this.#adapter.add(e));
return this;
}
public clear(): this {
this.#adapter.clear();
return this;
}
public clone(): CollectionCore<E, T> {
return new (this.constructor as any)(this.#adapter.value, this.#adapter);
}
public destroy(): this {
this.clear();
this.#adapter = null as any;
return this;
}
public delete(...element: E[]): boolean {
return element.every(e => this.#adapter.delete(e));
}
public forEach(callbackfn: (element: E, element2: E, collection: CollectionCore<E, T>) => void, thisArg?: any): this {
this.#adapter.forEach(callbackfn as any, thisArg);
return this;
}
public has(element: E): boolean {
return this.#adapter.has(element);
}
public lock(): this {
this.#adapter.lock?.();
return this;
}
public set(value: T): this {
this.#adapter.set(value);
return this;

Copilot uses AI. Check for mistakes.
return element.every(e => this.#adapter.delete(e));
}
public forEach(callbackfn: (element: E, element2: E, collection: CollectionCore<E, T>) => void, thisArg?: any): this {
return this.#adapter.forEach(callbackfn as any, thisArg), this;
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

The comma operator usage makes this code harder to read and understand. Consider refactoring to separate statements for better clarity:

public forEach(callbackfn: (element: E, element2: E, collection: CollectionCore<E, T>) => void, thisArg?: any): this {
  this.#adapter.forEach(callbackfn as any, thisArg);
  return this;
}
Suggested change
return this.#adapter.forEach(callbackfn as any, thisArg), this;
this.#adapter.forEach(callbackfn as any, thisArg);
return this;

Copilot uses AI. Check for mistakes.
import { CollectionBase } from '@typescript-package/collection';
```

[`CollectionBase`](https://github.com/typescript-package/hooks/blob/main/src/core/lib/collection.base.ts)
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

Incorrect GitHub repository link. The URL points to typescript-package/hooks but should point to typescript-package/collection to match this repository.

Suggested change
[`CollectionBase`](https://github.com/typescript-package/hooks/blob/main/src/core/lib/collection.base.ts)
[`CollectionBase`](https://github.com/typescript-package/collection/blob/main/src/core/lib/collection.base.ts)

Copilot uses AI. Check for mistakes.
}

public clear(): this {
return (this.collection as unknown as Set<Element>).clear(), this;
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

The comma operator usage makes this code harder to read and understand. Consider refactoring to separate statements for better clarity:

public clear(): this {
  (this.collection as unknown as Set<Element>).clear();
  return this;
}
Suggested change
return (this.collection as unknown as Set<Element>).clear(), this;
(this.collection as unknown as Set<Element>).clear();
return this;

Copilot uses AI. Check for mistakes.
}

public forEach(callbackfn: (element: Element, element2: Element, collection: CollectionAdapter<Element, Type>) => void, thisArg?: any): this {
return (this.collection as unknown as Set<Element>).forEach((value, value2) => callbackfn.call(thisArg, value, value2, this as any)), this;
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

The comma operator usage makes this code harder to read and understand. Consider refactoring to separate statements for better clarity:

public forEach(callbackfn: (element: Element, element2: Element, collection: CollectionAdapter<Element, Type>) => void, thisArg?: any): this {
  (this.collection as unknown as Set<Element>).forEach((value, value2) => callbackfn.call(thisArg, value, value2, this as any));
  return this;
}
Suggested change
return (this.collection as unknown as Set<Element>).forEach((value, value2) => callbackfn.call(thisArg, value, value2, this as any)), this;
(this.collection as unknown as Set<Element>).forEach((value, value2) => {
callbackfn.call(thisArg, value, value2, this as any);
});
return this;

Copilot uses AI. Check for mistakes.
}

public set(value: Type): this {
return (this.#collection = value), this;
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

The comma operator usage makes this code harder to read and understand. Consider refactoring to separate statements for better clarity:

public set(value: Type): this {
  this.#collection = value;
  return this;
}
Suggested change
return (this.#collection = value), this;
this.#collection = value;
return this;

Copilot uses AI. Check for mistakes.
Comment on lines +66 to +68
return this.clear(),
this.#adapter = null as any,
this;
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

The comma operator usage makes this code harder to read and understand. Consider refactoring to separate statements for better clarity:

public destroy(): this {
  this.clear();
  this.#adapter = null as any;
  return this;
}
Suggested change
return this.clear(),
this.#adapter = null as any,
this;
this.clear();
this.#adapter = null as any;
return this;

Copilot uses AI. Check for mistakes.
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.

2 participants