ronantakizawa/codereview-bench huggingface dataset:
Repository: angular/angular
File: packages/forms/signals/test/web/field_directive.spec.ts (TypeScript)

Code before:
    @Component({
      template: `
        <my-input [field]="f" />
      `,
      imports: [CustomInput, Field],
    })
    class PendingTestCmp {
      myInput = viewChild.required<CustomInput>(CustomInput);
      data = signal('test');
      f = form(this.data, (p) => {
        validateAsync(p, {
          params: () => [],
          factory: (params) =>
            resource({
              params,
              loader: () => new Promise<ValidationError[]>(() => {}),
            }),
          onSuccess: (results) => results,
          onError: () => null,
        });
      });
    }

    const comp = act(() => TestBed.createComponent(PendingTestCmp)).componentInstance;
    await TestBed.tick();

    expect(comp.myInput().pending()).toBe(true);
  });

  it(`should mark field as touched on native control 'blur' event`, () => {
    @Component({
      imports: [Field],
      template: `<input [field]="f">`,
    })
    class TestCmp {
      f = form(signal(''));
    }

    const fixture = act(() => TestBed.createComponent(TestCmp));
    const input = fixture.nativeElement.firstChild as HTMLInputElement;
    const field = fixture.componentInstance.f;

    expect(field().touched()).toBe(false);

    act(() => {
      input.dispatchEvent(new Event('blur'));
    });

    expect(field().touched()).toBe(true);
  });


Review comment (suggestion):
Could we update the test to use `promiseWithResolvers()` (see existing usage in this file) to test that the pending state is false after the validator resolves?

```ts
const {promise, resolve} = promiseWithResolvers<void>();
// Setup ...
expect(comp.myInput().pending()).toBe(true);

resolve();
await promise;
expect(comp.myInput().pending()).toBe(false);
```

Code after:

    @Component({
      template: `
        <my-input [field]="f" />
      `,
      imports: [CustomInput, Field],
    })
    class PendingTestCmp {
      myInput = viewChild.required<CustomInput>(CustomInput);
      data = signal('test');
      f = form(this.data, (p) => {
        validateAsync(p, {
          params: () => [],
          factory: (params) =>
            resource({
              params,
              loader: () => promise,
            }),
          onSuccess: (results) => results,
          onError: () => null,
        });
      });
    }

    const fix = act(() => TestBed.createComponent(PendingTestCmp));

    expect(fix.componentInstance.myInput().pending()).toBe(true);

    resolve([]);
    await promise;
    await fix.whenStable();
    expect(fix.componentInstance.myInput().pending()).toBe(false);
  });

  it(`should mark field as touched on native control 'blur' event`, () => {
    @Component({
      imports: [Field],
      template: `<input [field]="f">`,
    })
    class TestCmp {
      f = form(signal(''));
    }

    const fixture = act(() => TestBed.createComponent(TestCmp));
    const input = fixture.nativeElement.firstChild as HTMLInputElement;
    const field = fixture.componentInstance.f;

    expect(field().touched()).toBe(false);

    act(() => {
      input.dispatchEvent(new Event('blur'));
Repository: angular/angular
File: packages/forms/signals/test/web/field_directive.spec.ts (TypeScript)

Code before:

    @Component({
      template: `
        <my-input [field]="f" />
      `,
      imports: [CustomInput, Field],
    })
    class PendingTestCmp {
      myInput = viewChild.required<CustomInput>(CustomInput);
      data = signal('test');
      f = form(this.data, (p) => {
        validateAsync(p, {
          params: () => [],
          factory: (params) =>
            resource({
              params,
              loader: () => promise,
            }),
          onSuccess: (results) => results,
          onError: () => null,
        });
      });
    }

    const comp = act(() => TestBed.createComponent(PendingTestCmp)).componentInstance;
    await TestBed.tick();

    expect(comp.myInput().pending()).toBe(true);

    resolve([]);
    await promise;
    expect(comp.myInput().pending()).toBe(false);
  });

  it(`should mark field as touched on native control 'blur' event`, () => {
    @Component({
      imports: [Field],
      template: `<input [field]="f">`,
    })
    class TestCmp {
      f = form(signal(''));
    }

    const fixture = act(() => TestBed.createComponent(TestCmp));
    const input = fixture.nativeElement.firstChild as HTMLInputElement;
    const field = fixture.componentInstance.f;

    expect(field().touched()).toBe(false);

    act(() => {
      input.dispatchEvent(new Event('blur'));

Review comment (suggestion):
Can you confirm if this line is indeed still necessary? And if so, I think `whenStable()` is preferred for awaiting an async operation:

```ts
const fixture = TestBed.createComponent(PendingTestCmp);
await fixture.whenStable();
```

Code after:

    @Component({
      template: `
        <my-input [field]="f" />
      `,
      imports: [CustomInput, Field],
    })
    class PendingTestCmp {
      myInput = viewChild.required<CustomInput>(CustomInput);
      data = signal('test');
      f = form(this.data, (p) => {
        validateAsync(p, {
          params: () => [],
          factory: (params) =>
            resource({
              params,
              loader: () => promise,
            }),
          onSuccess: (results) => results,
          onError: () => null,
        });
      });
    }

    const fix = act(() => TestBed.createComponent(PendingTestCmp));

    expect(fix.componentInstance.myInput().pending()).toBe(true);

    resolve([]);
    await promise;
    await fix.whenStable();
    expect(fix.componentInstance.myInput().pending()).toBe(false);
  });

  it(`should mark field as touched on native control 'blur' event`, () => {
    @Component({
      imports: [Field],
      template: `<input [field]="f">`,
    })
    class TestCmp {
      f = form(signal(''));
    }

    const fixture = act(() => TestBed.createComponent(TestCmp));
    const input = fixture.nativeElement.firstChild as HTMLInputElement;
    const field = fixture.componentInstance.f;

    expect(field().touched()).toBe(false);

    act(() => {
      input.dispatchEvent(new Event('blur'));
Repository: angular/angular
File: adev/shared-docs/interfaces/code-example.ts (TypeScript)

Code before:
  title?: string;
  /** Name of the file. */
  name: string;
  /** Content of code snippet */
  sanitizedContent: SafeHtml;
  /** Text in following format `start-end`. Start and end are numbers, based on them provided range of lines will be displayed in collapsed mode  */
  visibleLinesRange?: string;

  shell?: boolean;
}

export interface ExampleMetadata {
  /** Numeric id of example, used to generate unique link to the example */
  id: number;
  /** Title of the example. */
  title?: string;
  /** Path to the preview component */
  path?: string;
  /** List of files which are part of the example. */
  files: Snippet[];
  /** True when ExampleViewer should have preview */
  preview: boolean;
  /** Whether to hide code example by default. */
  hideCode: boolean;
  /** Visual style for the code example */
  style: 'prefer' | 'avoid' | undefined;
}


Review comment (suggestion):
Shouldn't this be:
```suggestion
  style?: 'prefer' | 'avoid';
```

Code after:
  title?: string;
  /** Name of the file. */
  name: string;
  /** Content of code snippet */
  sanitizedContent: SafeHtml;
  /** Text in following format `start-end`. Start and end are numbers, based on them provided range of lines will be displayed in collapsed mode  */
  visibleLinesRange?: string;

  shell?: boolean;
}

export interface ExampleMetadata {
  /** Numeric id of example, used to generate unique link to the example */
  id: number;
  /** Title of the example. */
  title?: string;
  /** Path to the preview component */
  path?: string;
  /** List of files which are part of the example. */
  files: Snippet[];
  /** True when ExampleViewer should have preview */
  preview: boolean;
  /** Whether to hide code example by default. */
  hideCode: boolean;
  /** Visual style for the code example */
  style?: 'prefer' | 'avoid';
}
Repository: angular/angular
File: packages/core/src/render3/def_getters.ts (TypeScript)

Code before:
  if (!def) {
    throw new RuntimeError(
      RuntimeErrorCode.MISSING_DIRECTIVE_DEFINITION,
      (typeof ngDevMode === 'undefined' || ngDevMode) &&
        `Type ${stringify(type)} does not have 'ɵdir' property.`,
    );
  }
  return def;
}

export function getDirectiveDef<T>(type: any): DirectiveDef<T> | null {
  assertTypeDefined(type, '@Directive');
  return type[NG_DIR_DEF] || null;
}

export function getPipeDef<T>(type: any): PipeDef<T> | null {
  assertTypeDefined(type, '@Pipe');
  return type[NG_PIPE_DEF] || null;
}

function assertTypeDefined(type: any, symbolType: string): void {
  if (type == null) {
    throw new RuntimeError(
      RuntimeErrorCode.DEF_TYPE_UNDEFINED,
      (typeof ngDevMode === 'undefined' || ngDevMode) &&
        `Cannot read ${symbolType} metadata from "${type}". This can indicate a runtime ` +
          `circular dependency in your app that needs to be resolved.`,
    );
  }
}

/**
 * Checks whether a given Component, Directive or Pipe is marked as standalone.
 * This will return false if passed anything other than a Component, Directive, or Pipe class
 * See [this guide](guide/components/importing) for additional information:
 *
 * @param type A reference to a Component, Directive or Pipe.
 * @publicApi
 */
export function isStandalone(type: Type<unknown>): boolean {
  const def = getComponentDef(type) || getDirectiveDef(type) || getPipeDef(type);
  return def !== null && def.standalone;
}


Review comment (suggestion):
The inclusion of `type` here may not be super valuable given that it will be `undefined` or `null`. 

Code after:
  if (!def) {
    throw new RuntimeError(
      RuntimeErrorCode.MISSING_DIRECTIVE_DEFINITION,
      (typeof ngDevMode === 'undefined' || ngDevMode) &&
        `Type ${stringify(type)} does not have 'ɵdir' property.`,
    );
  }
  return def;
}

export function getDirectiveDef<T>(type: any): DirectiveDef<T> | null {
  assertTypeDefined(type, '@Directive');
  return type[NG_DIR_DEF] || null;
}

export function getPipeDef<T>(type: any): PipeDef<T> | null {
  assertTypeDefined(type, '@Pipe');
  return type[NG_PIPE_DEF] || null;
}

function assertTypeDefined(type: any, symbolType: string): void {
  if (type == null) {
    throw new RuntimeError(
      RuntimeErrorCode.DEF_TYPE_UNDEFINED,
      (typeof ngDevMode === 'undefined' || ngDevMode) &&
        `Cannot read ${symbolType} metadata. This can indicate a runtime ` +
          `circular dependency in your app that needs to be resolved.`,
    );
  }
}

/**
 * Checks whether a given Component, Directive or Pipe is marked as standalone.
 * This will return false if passed anything other than a Component, Directive, or Pipe class
 * See [this guide](guide/components/importing) for additional information:
 *
 * @param type A reference to a Component, Directive or Pipe.
 * @publicApi
 */
export function isStandalone(type: Type<unknown>): boolean {
  const def = getComponentDef(type) || getDirectiveDef(type) || getPipeDef(type);
  return def !== null && def.standalone;
}
Repository: angular/angular
File: adev/shared-docs/pipeline/api-gen/rendering/transforms/block-transforms.mts (TypeScript)

Code before:
/*!
 * @license
 * Copyright Google LLC All Rights Reserved.
 *
 * Use of this source code is governed by an MIT-style license that can be
 * found in the LICENSE file at https://angular.dev/license
 */

import {DocEntry} from '../entities.mjs';
import {BlockEntryRenderable} from '../entities/renderables.mjs';
import {
  addHtmlAdditionalLinks,
  addHtmlDescription,
  addHtmlJsDocTagComments,
  addHtmlUsageNotes,
  setEntryFlags,
} from './jsdoc-transforms.mjs';
import {addModuleName} from './module-name.mjs';
import {addRepo} from './repo.mjs';

/** Given an unprocessed block entry, get the fully renderable block entry. */
export async function getBlockRenderable(
  blockEntry: DocEntry,
  moduleName: string,
  repo: string,
): Promise<BlockEntryRenderable> {
  return setEntryFlags(
    addHtmlAdditionalLinks(
      addHtmlDescription(
        addHtmlUsageNotes(
          addHtmlJsDocTagComments(addRepo(addModuleName(blockEntry, moduleName), repo)),
        ),
      ),
    ),
  );
}


Review comment (suggestion):
nit: There is nothing async here. 
```suggestion
export function getBlockRenderable(
  blockEntry: DocEntry,
  moduleName: string,
  repo: string,
): BlockEntryRenderable {
```

Code after:
/*!
 * @license
 * Copyright Google LLC All Rights Reserved.
 *
 * Use of this source code is governed by an MIT-style license that can be
 * found in the LICENSE file at https://angular.dev/license
 */

import {DocEntry} from '../entities.mjs';
import {BlockEntryRenderable} from '../entities/renderables.mjs';
import {
  addHtmlAdditionalLinks,
  addHtmlDescription,
  addHtmlJsDocTagComments,
  addHtmlUsageNotes,
  setEntryFlags,
} from './jsdoc-transforms.mjs';
import {addModuleName} from './module-name.mjs';
import {addRepo} from './repo.mjs';

/** Given an unprocessed block entry, get the fully renderable block entry. */
export function getBlockRenderable(
  blockEntry: DocEntry,
  moduleName: string,
  repo: string,
): BlockEntryRenderable {
  return setEntryFlags(
    addHtmlAdditionalLinks(
      addHtmlDescription(
        addHtmlUsageNotes(
          addHtmlJsDocTagComments(addRepo(addModuleName(blockEntry, moduleName), repo)),
        ),
      ),
    ),
  );
}
Repository: angular/angular
File: devtools/projects/protocol/src/lib/messages.ts (TypeScript)

Code before:

  highlightComponent: (id: number) => void;
  selectComponent: (id: number) => void;
  removeComponentHighlight: () => void;

  enableTimingAPI: () => void;
  disableTimingAPI: () => void;

  // todo: type properly
  getInjectorProviders: (injector: SerializedInjector) => void;
  latestInjectorProviders: (
    injector: SerializedInjector,
    providers: SerializedProviderRecord[],
  ) => void;

  logProvider: (injector: SerializedInjector, providers: SerializedProviderRecord) => void;

  getTransferState: () => void;
  transferStateData: (data: Record<string, TransferStateValue> | null) => void;

  contentScriptConnected: (frameId: number, name: string, url: string) => void;
  contentScriptDisconnected: (frameId: number, name: string, url: string) => void;
  enableFrameConnection: (frameId: number, tabId: number) => void;
  frameConnected: (frameId: number) => void;
  detectAngular: (detectionResult: AngularDetection) => void;
  backendReady: () => void;

  log: (logEvent: {message: string; level: 'log' | 'warn' | 'debug' | 'error'}) => void;
}


Review comment (suggestion):
Question: Is this used anywhere? I don't see `backendInstalled` invoked as a function and I'm not sure what that would means returning `void`.

Code after:

  highlightComponent: (id: number) => void;
  selectComponent: (id: number) => void;
  removeComponentHighlight: () => void;

  enableTimingAPI: () => void;
  disableTimingAPI: () => void;

  // todo: type properly
  getInjectorProviders: (injector: SerializedInjector) => void;
  latestInjectorProviders: (
    injector: SerializedInjector,
    providers: SerializedProviderRecord[],
  ) => void;

  logProvider: (injector: SerializedInjector, providers: SerializedProviderRecord) => void;

  getTransferState: () => void;
  transferStateData: (data: Record<string, TransferStateValue> | null) => void;

  contentScriptConnected: (frameId: number, name: string, url: string) => void;
  contentScriptDisconnected: (frameId: number, name: string, url: string) => void;
  enableFrameConnection: (frameId: number, tabId: number) => void;
  frameConnected: (frameId: number) => void;
  detectAngular: (detectionResult: AngularDetection) => void;
  backendInstalled: () => void;
  backendReady: () => void;

  log: (logEvent: {message: string; level: 'log' | 'warn' | 'debug' | 'error'}) => void;
}
Repository: angular/angular
File: devtools/projects/shell-browser/src/app/content-script.ts (TypeScript)

Code before:
 */

/// <reference types="chrome"/>

import {ChromeMessageBus} from './chrome-message-bus';
import {BACKEND_URI, CONTENT_SCRIPT_URI, DETECT_ANGULAR_SCRIPT_URI} from './communication';
import {SamePageMessageBus} from './same-page-message-bus';

let backgroundDisconnected = false;
let backendInstalled = false;
let backendInitialized = false;

const port = chrome.runtime.connect({
  name: `${document.title || location.href}`,
});

const handleDisconnect = (): void => {
  // console.log('Background disconnected', new Date());
  localMessageBus.emit('shutdown');
  localMessageBus.destroy();
  chromeMessageBus.destroy();
  backgroundDisconnected = true;
};

port.onDisconnect.addListener(handleDisconnect);

const detectAngularMessageBus = new SamePageMessageBus(
  CONTENT_SCRIPT_URI,
  DETECT_ANGULAR_SCRIPT_URI,
);

detectAngularMessageBus.on('detectAngular', (detectionResult) => {
  // only install backend once
  if (backendInstalled) {
    return;
  }

  if (detectionResult.isAngularDevTools !== true) {
    return;
  }

  if (detectionResult.isAngular !== true) {
    return;
  }

  // Defensive check against non html page. Realistically this should never happen.
  if (document.contentType !== 'text/html') {
    return;
  }

  // Inform the background page so it can toggle the popup and icon.

Review comment (bug):
Should we use `debug` instead ? 
```suggestion
    console.debug('Attempting handshake with backend', new Date());
```
This will prevent spaming the console by default. 

Code after:
 */

/// <reference types="chrome"/>

import {ChromeMessageBus} from './chrome-message-bus';
import {BACKEND_URI, CONTENT_SCRIPT_URI, DETECT_ANGULAR_SCRIPT_URI} from './communication';
import {SamePageMessageBus} from './same-page-message-bus';

let backgroundDisconnected = false;
let backendInitialized = false;

const port = chrome.runtime.connect({
  name: `${document.title || location.href}`,
});

const handleDisconnect = (): void => {
  localMessageBus.emit('shutdown');
  localMessageBus.destroy();
  chromeMessageBus.destroy();
  backgroundDisconnected = true;
};

function attemptBackendHandshake() {
  if (!backendInitialized) {
    // tslint:disable-next-line:no-console
    console.log('Attempting handshake with backend', new Date());

    const retry = () => {
      if (backendInitialized || backgroundDisconnected) {
        return;
      }
      handshakeWithBackend();
      setTimeout(retry, 500);
    };
    retry();
  }
}

port.onDisconnect.addListener(handleDisconnect);

const detectAngularMessageBus = new SamePageMessageBus(
  CONTENT_SCRIPT_URI,
  DETECT_ANGULAR_SCRIPT_URI,
);

detectAngularMessageBus.on('detectAngular', (detectionResult) => {
  if (detectionResult.isAngularDevTools !== true) {
    return;
  }

  if (detectionResult.isAngular !== true) {
Repository: angular/angular
File: packages/core/src/application/stability_debug.ts (TypeScript)

Code before:
import {
  EnvironmentProviders,
  inject,
  makeEnvironmentProviders,
  ENVIRONMENT_INITIALIZER,
} from '../di';
import {PendingTasksInternal} from '../pending_tasks_internal';
import {NgZone} from '../zone';
import {provideAppInitializer} from './application_init';
import {ApplicationRef} from './application_ref';

/**
 * Provides an application initializer that will log information about what tasks are keeping
 * the application from stabilizing if the application does not stabilize within 9 seconds.
 *
 * The logged information includes the stack of the tasks preventing stability. This stack can be traced
 * back to the source in the application code.
 *
 * If you are using Zone.js, it is recommended that you also temporarily import "zone.js/plugins/task-tracking".
 * This Zone.js plugin provides additional information about which macrotasks are scheduled in the Angular Zone
 * and keeping the Zone from stabilizing.
 *
 * @usage
 *
 * ```ts
 * import 'zone.js/plugins/task-tracking';
 *
 * bootstrapApplication(AppComponent, {providers: [provideStabilityDebugging()]});
 * ```
 *
 * IMPORTANT: Neither the zone.js task tracking plugin nor this utility are removed from production bundles.
 * They are intended for temporary use while debugging stability issues during development, including for
 * optimized production builds.
 *
 * @publicApi 21.1
 */
export function provideStabilityDebugging(): EnvironmentProviders {
  const openTasks = new Map<number, Error>();
  return makeEnvironmentProviders([
    {
      provide: ENVIRONMENT_INITIALIZER,
      multi: true,
      useFactory: () => {
        const pendingTasks = inject(PendingTasksInternal);
        pendingTasks.debugTaskTracker = {
          add(taskId: number): void {
            openTasks.set(taskId, new Error('Task stack tracking error'));
          },

Review comment (suggestion):
fwiw we only support `'remarks', 'usageNotes', 'example'` 
https://github.com/angular/angular/blob/main/adev/shared-docs/pipeline/api-gen/rendering/transforms/jsdoc-transforms.mts#L37 
```suggestion
 * @usageNotes
```

Code after:
/**
 * @license
 * Copyright Google LLC All Rights Reserved.
 *
 * Use of this source code is governed by an MIT-style license that can be
 * found in the LICENSE file at https://angular.dev/license
 */

import {InjectionToken} from '../di/injection_token';

export const DEBUG_TASK_TRACKER = new InjectionToken<DebugTaskTracker>(
  typeof ngDevMode !== 'undefined' && ngDevMode ? 'DEBUG_TASK_TRACKER' : '',
);
export interface DebugTaskTracker {
  add(taskId: number): void;
  remove(taskId: number): void;
}
Repository: angular/angular
File: packages/core/src/application/stability_debug.ts (TypeScript)

Code before:
        ngZone.run(() => {
          _taskTrackingZone = Zone.current.get('TaskTrackingZone');
        });
      }
      ngZone.runOutsideAngular(() => {
        const timeoutId = setTimeout(() => {
          console.debug('---- Application did not stabilize within 9 seconds ----');
          if (typeof Zone !== 'undefined' && !_taskTrackingZone) {
            console.info(
              'Zone.js is present but no TaskTrackingZone found. To enable better debugging of tasks in the Angular Zone, ' +
                'import "zone.js/plugins/task-tracking" in your application.',
            );
          }
          if (_taskTrackingZone?.macroTasks?.length) {
            console.group('Macrotasks keeping Angular Zone unstable:');
            for (const t of _taskTrackingZone?.macroTasks ?? []) {
              console.debug(t.creationLocation.stack);
            }
            console.groupEnd();
          }
          console.group('PendingTasks keeping application unstable:');
          for (const error of openTasks.values()) {
            console.debug(error.stack);
          }
          console.groupEnd();
        }, 9_000);

        applicationRef.whenStable().then(() => {
          clearTimeout(timeoutId);
        });
      });
    }),
  ]);
}


Review comment (bug):
Nit: Any particular reasoning behind 9s beyond hydration? Should this be configurable? If SSR takes >1s, that's probably a bug on its own and I could imagine teams wanting to enforce a stricter deadline (though maybe this is the wrong mechanism for that).

Code after:
/**
 * @license
 * Copyright Google LLC All Rights Reserved.
 *
 * Use of this source code is governed by an MIT-style license that can be
 * found in the LICENSE file at https://angular.dev/license
 */

import {InjectionToken} from '../di/injection_token';

export const DEBUG_TASK_TRACKER = new InjectionToken<DebugTaskTracker>(
  typeof ngDevMode !== 'undefined' && ngDevMode ? 'DEBUG_TASK_TRACKER' : '',
);
export interface DebugTaskTracker {
  add(taskId: number): void;
  remove(taskId: number): void;
}