ronantakizawa/codereview-bench huggingface dataset:
Repository: pixijs/pixijs
File: src/utils/pool/Pool.ts (TypeScript)

Code before:
     * @readonly
     * @member {number}
     */
    get totalSize(): number
    {
        return this._count;
    }

    /**
     * Gets the number of items in the pool that are free to use without needing to create more.
     * @readonly
     * @member {number}
     */
    get totalFree(): number
    {
        return this._pool.length;
    }

    /**
     * Gets the number of items in the pool that are currently in use.
     * @readonly
     * @member {number}
     */
    get totalUsed(): number
    {
        return this._count - this._pool.length;
    }
}

/**
 * An object that can be stored in a {@link utils.Pool}.
 * @memberof utils
 */
export type PoolItem = {
    init?: (data?: any) => void;
    reset?: () => void;
};

/**
 * The constructor of an object that can be stored in a {@link utils.Pool}.
 * @typeParam K - The type of the object that can be stored in a {@link utils.Pool}.
 * @memberof utils
 */
export type PoolItemConstructor<K extends PoolItem> = new () => K;


Review comment (suggestion):
```suggestion
        return this._count - this._index;
```

Code after:
     * @readonly
     * @member {number}
     */
    get totalSize(): number
    {
        return this._count;
    }

    /**
     * Gets the number of items in the pool that are free to use without needing to create more.
     * @readonly
     * @member {number}
     */
    get totalFree(): number
    {
        return this._index;
    }

    /**
     * Gets the number of items in the pool that are currently in use.
     * @readonly
     * @member {number}
     */
    get totalUsed(): number
    {
        return this._count - this._index;
    }
}

/**
 * An object that can be stored in a {@link utils.Pool}.
 * @memberof utils
 */
export type PoolItem = {
    init?: (data?: any) => void;
    reset?: () => void;
};

/**
 * The constructor of an object that can be stored in a {@link utils.Pool}.
 * @typeParam K - The type of the object that can be stored in a {@link utils.Pool}.
 * @memberof utils
 */
export type PoolItemConstructor<K extends PoolItem> = new () => K;
Repository: pixijs/pixijs
File: src/utils/sayHello.ts (TypeScript)

Code before:
import { DOMAdapter } from '../environment/adapter';

let saidHello = false;

export const VERSION = '$_VERSION';

/**
 * Prints out the version and renderer information for this running instance of PIXI.
 * @param type - The name of the renderer this instance is using.
 * @returns {void}
 */
export function sayHello(type: string): void
{
    if (saidHello)
    {
        return;
    }

    if (DOMAdapter.get().getNavigator().userAgent.toLowerCase().indexOf('chrome') > -1)
    {
        const args = [
            `%c  %c  %c  %c  %c PixiJS %c v${VERSION} (${type}) http://www.pixijs.com/\n\n`,
            'background: #E72264; padding:5px 0;',
            'background: #6CA2EA; padding:5px 0;',
            'background: #B5D33D; padding:5px 0;',
            'background: #FED23F; padding:5px 0;',
            'color: #FFFFFF; background: #E72264; padding:5px 0;',
            'color: #E72264; background: #FFFFFF; padding:5px 0;',
        ];

        globalThis.console.log(...args);
    }
    else if (globalThis.console)

Review comment (suggestion):
```suggestion
 * Prints out the version and renderer information for this running instance of PixiJS.
```

Code after:
import { DOMAdapter } from '../environment/adapter';

let saidHello = false;

export const VERSION = '$_VERSION';

/**
 * Prints out the version and renderer information for this running instance of PixiJS.
 * @param type - The name of the renderer this instance is using.
 * @returns {void}
 */
export function sayHello(type: string): void
{
    if (saidHello)
    {
        return;
    }

    if (DOMAdapter.get().getNavigator().userAgent.toLowerCase().indexOf('chrome') > -1)
    {
        const args = [
            `%c  %c  %c  %c  %c PixiJS %c v${VERSION} (${type}) http://www.pixijs.com/\n\n`,
            'background: #E72264; padding:5px 0;',
            'background: #6CA2EA; padding:5px 0;',
            'background: #B5D33D; padding:5px 0;',
            'background: #FED23F; padding:5px 0;',
            'color: #FFFFFF; background: #E72264; padding:5px 0;',
            'color: #E72264; background: #FFFFFF; padding:5px 0;',
        ];

        globalThis.console.log(...args);
    }
    else if (globalThis.console)
Repository: pixijs/pixijs
File: src/scene/graphics/shared/utils/convertFillInputToFillStyle.ts (TypeScript)

Code before:
            color: 0xffffff,
            texture: gradient.texture,
            matrix: gradient.transform,
        };
    }

    const style: FillStyle = { ...defaultStyle, ...(value as FillStyle) };

    if (style.texture)
    {
        if (style.texture !== Texture.WHITE)
        {
            const m = style.matrix || new Matrix();

            m.scale(
                1 / style.texture.frame.width,
                1 / style.texture.frame.height
            );

            style.matrix = m;
        }

        // ensure texture style address mode is set to "repeat" or "mirror-repeat"
        const sourceStyle = style.texture.source.style;

        if (sourceStyle.addressMode !== 'repeat' && sourceStyle.addressMode !== 'mirror-repeat')
        {
            sourceStyle.addressMode = 'repeat';
        }
    }

    const color = Color.shared.setValue(style.color);

    style.alpha *= color.alpha;
    style.color = color.toNumber();

    // its a regular fill style!
    return style as ConvertedFillStyle;
}


Review comment (suggestion):
```suggestion
        if (sourceStyle.addressMode === 'clamp-to-edge')
```

Code after:
            color: 0xffffff,
            texture: gradient.texture,
            matrix: gradient.transform,
        };
    }

    const style: FillStyle = { ...defaultStyle, ...(value as FillStyle) };

    if (style.texture)
    {
        if (style.texture !== Texture.WHITE)
        {
            const m = style.matrix || new Matrix();

            m.scale(
                1 / style.texture.frame.width,
                1 / style.texture.frame.height
            );

            style.matrix = m;
        }

        const sourceStyle = style.texture.source.style;

        if (sourceStyle.addressMode === 'clamp-to-edge')
        {
            sourceStyle.addressMode = 'repeat';
        }
    }

    const color = Color.shared.setValue(style.color);

    style.alpha *= color.alpha;
    style.color = color.toNumber();

    // its a regular fill style!
    return style as ConvertedFillStyle;
}
Repository: pixijs/pixijs
File: src/rendering/renderers/shared/texture/Texture.ts (TypeScript)

Code before:
            this.frame.width = this._source.width;
            this.frame.height = this._source.height;
        }

        this.updateUvs();
        this.emit('update', this);
    }

    /** @deprecated since 8.0.0 */
    get baseTexture(): TextureSource
    {
        deprecation(v8_0_0, 'Texture.baseTexture is now Texture.source');

        return this._source;
    }

    /**
     * an Empty Texture used internally by the engine
     * @ignore
     * @internal
     */
    public static EMPTY: Texture;
    /**
     * a White texture used internally by the engine
     * @ignore
     * @internal
     */
    public static WHITE: Texture;
}

Texture.EMPTY = new Texture({

});

Texture.EMPTY.label = 'EMPTY';
Texture.EMPTY.destroy = NOOP;


Review comment (suggestion):
```suggestion
```

WHITE and EMPTY are used commonly so should not be ignored

Code after:
            this.frame.width = this._source.width;
            this.frame.height = this._source.height;
        }

        this.updateUvs();
        this.emit('update', this);
    }

    /** @deprecated since 8.0.0 */
    get baseTexture(): TextureSource
    {
        deprecation(v8_0_0, 'Texture.baseTexture is now Texture.source');

        return this._source;
    }

    /** an Empty Texture used internally by the engine */
    public static EMPTY: Texture;
    /** a White texture used internally by the engine */
    public static WHITE: Texture;
}

Texture.EMPTY = new Texture({

});

Texture.EMPTY.label = 'EMPTY';
Texture.EMPTY.destroy = NOOP;
Repository: pixijs/pixijs
File: src/scene/sprite-tiling/TilingSprite.ts (TypeScript)

Code before:

        return new TilingSprite({
            texture: source,
            ...options,
        });
    }

    /**
     * @param options - The options for creating the tiling sprite.
     */
    constructor(options?: Texture | TilingSpriteOptions)
    {
        if (options instanceof Texture)
        {
            options = { texture: options };
        }

        const { texture, width, height, applyAnchorToTexture, ...rest } = options ?? {};

        super({
            view: new TilingSpriteView(definedProps({
                texture,
                width,
                height,
                applyAnchorToTexture,
            })),
            label: 'TilingSprite',
            ...rest
        });

        this.allowChildren = false;
    }

    /**
     * Changes frame clamping in corresponding textureMatrix
     * Change to -0.5 to add a pixel to the edge, recommended for transparent trimmed textures in atlas
     * @default 0.5
     * @member {number}
     */
    get clampMargin()
    {
        return this.view.texture.textureMatrix.clampMargin;
    }

    set clampMargin(value: number)
    {
        this.view.texture.textureMatrix.clampMargin = value;
    }

    set texture(value)
    {

Review comment (bug):
@Zyie is `if (process.env.DEBUG)` a thing around logs and deprecation warns like in v7?

Code after:
        }

        return new TilingSprite({
            texture: source,
            ...options,
        });
    }

    /**
     * @param options - The options for creating the tiling sprite.
     */
    constructor(options?: Texture | TilingSpriteOptions);
    /** @deprecated since 8.0.0 */
    constructor(texture: Texture, width: number, height: number);
    constructor(...args: [(Texture | TilingSpriteViewOptions)?] | [Texture, number, number])
    {
        let options = args[0] || {};

        if (options instanceof Texture)
        {
            options = { texture: options };
        }

        if (args.length > 1)
        {
            deprecation(v8_0_0, 'use new TilingSprite({ texture, width:100, height:100 }) instead');

            options.width = args[1];
            options.height = args[2];
        }

        if (options instanceof Texture)
        {
            options = { texture: options };
        }

        const { texture, width, height, applyAnchorToTexture, ...rest } = options ?? {};

        super({
            view: new TilingSpriteView(definedProps({
                texture,
                width,
                height,
                applyAnchorToTexture,
            })),
            label: 'TilingSprite',
            ...rest
        });

        this.allowChildren = false;
    }
Repository: pixijs/pixijs
File: src/scene/graphics/shared/utils/convertFillInputToFillStyle.ts (TypeScript)

Code before:
            fill: fillStyleToParse.fill ?? null,
        };
    }

    // // TODO Texture
    else if (styleToMerge instanceof FillGradient)
    {
        const gradient = styleToMerge as FillGradient;

        gradient.buildLinearGradient();

        return {
            ...fillStyleToParse,
            color: 0xffffff,
            texture: gradient.texture,
            matrix: gradient.transform,
        };
    }

    const style: FillStyle = { ...defaultStyle, ...(value as FillStyle) };

    if (style.texture)
    {
        if (style.texture !== Texture.WHITE)
        {
            const m = style.matrix ? style.matrix.invert() : new Matrix();

            m.scale(
                1 / style.texture.frame.width,
                1 / style.texture.frame.height
            );

            style.matrix = m;
        }

        const sourceStyle = style.texture.source.style;

        if (sourceStyle.addressMode === 'clamp-to-edge')
        {
            sourceStyle.addressMode = 'repeat';
        }
    }

    const color = Color.shared.setValue(style.color);

    style.alpha *= color.alpha;
    style.color = color.toNumber();
    style.matrix = style.matrix ? style.matrix.clone() : null; // todo: lets optimise this!

    // its a regular fill style!
    return style as ConvertedFillStyle;

Review comment (suggestion):
```suggestion
            const m = style.matrix?.invert() || new Matrix();
```

Code after:
            fill: fillStyleToParse.fill ?? null,
        };
    }

    // // TODO Texture
    else if (styleToMerge instanceof FillGradient)
    {
        const gradient = styleToMerge as FillGradient;

        gradient.buildLinearGradient();

        return {
            ...fillStyleToParse,
            color: 0xffffff,
            texture: gradient.texture,
            matrix: gradient.transform,
        };
    }

    const style: FillStyle = { ...defaultStyle, ...(value as FillStyle) };

    if (style.texture)
    {
        if (style.texture !== Texture.WHITE)
        {
            const m = style.matrix?.invert() || new Matrix();

            m.scale(
                1 / style.texture.frame.width,
                1 / style.texture.frame.height
            );

            style.matrix = m;
        }

        const sourceStyle = style.texture.source.style;

        if (sourceStyle.addressMode === 'clamp-to-edge')
        {
            sourceStyle.addressMode = 'repeat';
        }
    }

    const color = Color.shared.setValue(style.color);

    style.alpha *= color.alpha;
    style.color = color.toNumber();
    style.matrix = style.matrix ? style.matrix.clone() : null; // todo: lets optimise this!

    // its a regular fill style!
    return style as ConvertedFillStyle;
Repository: pixijs/pixijs
File: packages/runner/src/Runner.ts (TypeScript)

Code before:
export type RunnerListenerCallback<ARG extends any[]> = (...args: ARG) => unknown;

export type ValidRunnerItem<T extends string, ARG extends unknown[] = any[]> =
    { [K in T]: RunnerListenerCallback<ARG> | unknown };

export type AnyRunnerItem = Record<string, unknown>;

export type EmptyRunnerItem = Record<string, never>;

export type RunnerItem<T, ARG extends unknown[] = any[]> =
    T extends string ?
        ValidRunnerItem<T, ARG> & AnyRunnerItem | EmptyRunnerItem :
        unknown;

/**
 * A Runner is a highly performant and simple alternative to signals. Best used in situations
 * where events are dispatched to many objects at high frequency (say every frame!)
 *
 * Like a signal:
 *
 * ```js
 * import { Runner } from '@pixi/runner';
 *
 * const myObject = {
 *     loaded: new Runner('loaded'),
 * };
 *
 * const listener = {
 *     loaded: function() {
 *         // Do something when loaded
 *     }
 * };
 *
 * myObject.loaded.add(listener);
 *
 * myObject.loaded.emit();
 * ```
 *

Review comment (suggestion):
Small suggestion to prefix all the types with "RunnerItem". Make them alphabetically sort together with auto-complete and in docs.

Also, is it necessary to export the Empty and Any types? Why not just inline those?

```suggestion
export type RunnerItemValid<T extends string, ARG extends unknown[] = any[]> =
    { [K in T]: RunnerListenerCallback<ARG> | unknown };

export type RunnerItemAny = Record<string, unknown>;

export type RunnerItemEmpty = Record<string, never>;

export type RunnerItem<T, ARG extends unknown[] = any[]> =
    T extends string ?
        RunnerItemValid<T, ARG> & RunnerItemAny | RunnerItemEmpty :
        unknown;
```

Code after:
export type RunnerListenerCallback<ARG extends unknown[] = any[]> = (...args: ARG) => unknown;

type RunnerItemValid<T extends string, ARG extends unknown[] = any[]> =
    { [K in T]: RunnerListenerCallback<ARG> | unknown };

type RunnerItemAny = Record<string, unknown>;

type RunnerItemEmpty = Record<string, never>;

export type RunnerItem<T = string, ARG extends unknown[] = any[]> =
    T extends string ?
        RunnerItemValid<T, ARG> & RunnerItemAny | RunnerItemEmpty :
        unknown;

/**
 * A Runner is a highly performant and simple alternative to signals. Best used in situations
 * where events are dispatched to many objects at high frequency (say every frame!)
 *
 * Like a signal:
 *
 * ```js
 * import { Runner } from '@pixi/runner';
 *
 * const myObject = {
 *     loaded: new Runner('loaded'),
 * };
 *
 * const listener = {
 *     loaded: function() {
 *         // Do something when loaded
 *     }
 * };
 *
 * myObject.loaded.add(listener);
 *
 * myObject.loaded.emit();
 * ```
 *
Repository: pixijs/pixijs
File: packages/runner/src/Runner.ts (TypeScript)

Code before:
 *
 * ```js
 * import { Runner } from '@pixi/runner';
 *
 * const myGame = {
 *     update: new Runner('update'),
 * };
 *
 * const gameObject = {
 *     update: function(time) {
 *         // Update my gamey state
 *     },
 * };
 *
 * myGame.update.add(gameObject);
 *
 * myGame.update.emit(time);
 * ```
 *
 * Type safety:
 *
 * ```ts
 *
 * let runner: Runner<'update', [number]>;
 *
 * // This won't work because the function name 'update' is expected
 * runner = new Runner('destroy');
 *
 * // This is fine
 * runner = new Runner('update');
 *
 * // This won't work because the number is expected
 * runner.emit("10");
 *
 * // This is fine
 * runner.emit(10);
 *
 * // This won't work because provided object does not contain 'update' key
 * runner.add({
 *     destroy: function() {
 *         // Destroy the game
 *     },
 * });
 *
 * // This is fine
 * runner.add({
 *     update: function(time) {
 *         // Update my gamey state
 *     },
 *     destroy: function() {
 *         // Destroy the game

Review comment (suggestion):
Minor suggestion for example code, otherwise this is a great example.

```suggestion
 * import { Runner } from '@pixi/runner';
 *
 * const runner: Runner<'update', [number]>;
```

Code after:
 *
 * ```js
 * import { Runner } from '@pixi/runner';
 *
 * const myGame = {
 *     update: new Runner('update'),
 * };
 *
 * const gameObject = {
 *     update: function(time) {
 *         // Update my gamey state
 *     },
 * };
 *
 * myGame.update.add(gameObject);
 *
 * myGame.update.emit(time);
 * ```
 *
 * Type safety:
 *
 * ```ts
 *
 * import { Runner } from '@pixi/runner';
 *
 * let runner: Runner<'update', [number]>;
 *
 * // This won't work because the function name 'update' is expected
 * runner = new Runner('destroy');
 *
 * // This is fine
 * runner = new Runner('update');
 *
 * // This won't work because the number is expected
 * runner.emit("10");
 *
 * // This is fine
 * runner.emit(10);
 *
 * // This won't work because provided object does not contain 'update' key
 * runner.add({
 *     destroy: function() {
 *         // Destroy the game
 *     },
 * });
 *
 * // This is fine
 * runner.add({
 *     update: function(time) {
 *         // Update my gamey state
 *     },
Repository: pixijs/pixijs
File: packages/runner/src/Runner.ts (TypeScript)

Code before:
 *
 * ```js
 * import { Runner } from '@pixi/runner';
 *
 * const myGame = {
 *     update: new Runner('update'),
 * };
 *
 * const gameObject = {
 *     update: function(time) {
 *         // Update my gamey state
 *     },
 * };
 *
 * myGame.update.add(gameObject);
 *
 * myGame.update.emit(time);
 * ```
 *
 * Type safety:
 *
 * ```ts
 *
 * import { Runner } from '@pixi/runner';
 *
 * const runner: Runner<'update', [number]>;
 *
 * // This won't work because the function name 'update' is expected
 * runner = new Runner('destroy');
 *
 * // This is fine
 * runner = new Runner('update');
 *
 * // This won't work because the number is expected
 * runner.emit("10");
 *
 * // This is fine
 * runner.emit(10);
 *
 * // This won't work because provided object does not contain 'update' key
 * runner.add({
 *     destroy: function() {
 *         // Destroy the game
 *     },
 * });
 *
 * // This is fine
 * runner.add({
 *     update: function(time) {
 *         // Update my gamey state
 *     },

Review comment (suggestion):
@bigtimebuddy we should keep 'let' here, I guess, because we re-define a runner down below in the following code, isn't it? 🤔

`let runner: Runner<'update', [number]>;`

Code after:
 *
 * ```js
 * import { Runner } from '@pixi/runner';
 *
 * const myGame = {
 *     update: new Runner('update'),
 * };
 *
 * const gameObject = {
 *     update: function(time) {
 *         // Update my gamey state
 *     },
 * };
 *
 * myGame.update.add(gameObject);
 *
 * myGame.update.emit(time);
 * ```
 *
 * Type safety:
 *
 * ```ts
 *
 * import { Runner } from '@pixi/runner';
 *
 * let runner: Runner<'update', [number]>;
 *
 * // This won't work because the function name 'update' is expected
 * runner = new Runner('destroy');
 *
 * // This is fine
 * runner = new Runner('update');
 *
 * // This won't work because the number is expected
 * runner.emit("10");
 *
 * // This is fine
 * runner.emit(10);
 *
 * // This won't work because provided object does not contain 'update' key
 * runner.add({
 *     destroy: function() {
 *         // Destroy the game
 *     },
 * });
 *
 * // This is fine
 * runner.add({
 *     update: function(time) {
 *         // Update my gamey state
 *     },
Repository: pixijs/pixijs
File: src/scene/container/bounds/Bounds.ts (TypeScript)

Code before:
import { Matrix } from '../../../maths/matrix/Matrix';
import { Rectangle } from '../../../maths/shapes/Rectangle';

/** Simple bounds implementation instead of more ambiguous [number, number, number, number] */
export interface BoundsData
{
    minX: number;
    minY: number;
    maxX: number;
    maxY: number;
}

const defaultMatrix = new Matrix();

// TODO optimisations
// 1 - get rectangle could use a dirty flag, rather than setting the data each time is called
// 62- getFrame ALWAYS assumes a matrix, could be optimised to avoid the matrix calculation if not needed
export class Bounds
{
    public minX = Infinity;

    public minY = Infinity;

    public maxX = -Infinity;

    public maxY = -Infinity;

    public matrix = defaultMatrix;

    private _rectangle: Rectangle;

    constructor(minX = Infinity, minY = Infinity, maxX = -Infinity, maxY = -Infinity)
    {
        this.minX = minX;
        this.minY = minY;
        this.maxX = maxX;
        this.maxY = maxY;
    }

    get rectangle(): Rectangle
    {
        if (!this._rectangle)

Review comment (suggestion):
```suggestion
// 2- getFrame ALWAYS assumes a matrix, could be optimised to avoid the matrix calculation if not needed
```

Code after:
import { Matrix } from '../../../maths/matrix/Matrix';
import { Rectangle } from '../../../maths/shapes/Rectangle';

/** Simple bounds implementation instead of more ambiguous [number, number, number, number] */
export interface BoundsData
{
    minX: number;
    minY: number;
    maxX: number;
    maxY: number;
}

const defaultMatrix = new Matrix();

// TODO optimisations
// 1 - get rectangle could use a dirty flag, rather than setting the data each time is called
// 2- getFrame ALWAYS assumes a matrix, could be optimised to avoid the matrix calculation if not needed
export class Bounds
{
    public minX = Infinity;

    public minY = Infinity;

    public maxX = -Infinity;

    public maxY = -Infinity;

    public matrix = defaultMatrix;

    private _rectangle: Rectangle;

    constructor(minX = Infinity, minY = Infinity, maxX = -Infinity, maxY = -Infinity)
    {
        this.minX = minX;
        this.minY = minY;
        this.maxX = maxX;
        this.maxY = maxY;
    }

    get rectangle(): Rectangle
    {
        if (!this._rectangle)