Skip to content

Conversation

artembelik
Copy link
Contributor

No description provided.

Copy link

@Copilot 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 applies the kbqInjectElementRef() utility function across the codebase to replace direct usage of inject(ElementRef). The change improves type safety by ensuring consistent ElementRef<HTMLElement> typing throughout the components.

Key changes:

  • Replaces inject(ElementRef) with kbqInjectElementRef() for proper HTMLElement typing
  • Updates public API signatures to reflect more specific return types
  • Includes complementary use of kbqInjectNativeElement() for direct native element access

Reviewed Changes

Copilot reviewed 56 out of 56 changed files in this pull request and generated 3 comments.

File Description
tools/public_api_guard/components/*.api.md Updates public API signatures to reflect proper HTMLElement typing for ElementRef properties
packages/components/*/**.ts Replaces ElementRef injection with kbqInjectElementRef() utility function
packages/docs-examples/components/*/**.ts Applies the same ElementRef utility replacement in example code
apps/docs/src/app/components/*/**.ts Updates documentation components to use the new utility function

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link

github-actions bot commented Sep 23, 2025

Visit the preview URL for this PR (updated for commit 8ef9208):

https://koobiq-next--prs-1028-c9le72t3.web.app

(expires Wed, 01 Oct 2025 10:53:42 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: c9e37e518febda70d0317d07e8ceb35ac43c534c

@lskramarov
Copy link
Contributor

Это лишняя обертка.

Там где вылезли ошибки с типами нужно добавить типы. Из-за одного места делать подобные вещи не стоит.

@artembelik
Copy link
Contributor Author

Это лишняя обертка.

я так не считаю, во первых эта запись читабельней

// before
readonly elementRef = inject<ElementRef<HTMLElement>>(ElementRef<HTMLElement>);
//after
readonly elementRef = kbqInjectElementRef();

во вторых она уже выявила несколько мест с потенциальными рантайм ошибками


подобный подход не редкость - пример

@lskramarov
Copy link
Contributor

lskramarov commented Sep 23, 2025

Это лишняя обертка.

я так не считаю, во первых эта запись читабельней

// before
readonly elementRef = inject<ElementRef<HTMLElement>>(ElementRef<HTMLElement>);
//after
readonly elementRef = kbqInjectElementRef();

во вторых она уже выявила несколько мест с потенциальными рантайм ошибками

подобный подход не редкость - пример

Я уже об этом говорил на примере оберток с провайдерами...

  1. Ты обернул и спрятал дженерики внутри kbqInjectElementRef и что бы понять что он делает, нужно смотреть реализацию и все те же типы.
  2. Ты добавляешь еще одну сущность вместо того что бы просто использовать имеющиеся типы.
  3. В приведенном примере обертка сделана что бы получать nativeElement из ElementRef (это не обертка ради типов).
  4. В аргументе дженерик не нужен, достаточно: inject<ElementRef<HTMLElement>>(ElementRef);
  5. Если следовать такому принципу то любое множество/сочетание типов можно обернуть в функции, тем самым породив еще большее количество сущностей с теми же типами

@artembelik artembelik marked this pull request as draft September 24, 2025 08:31
@artembelik artembelik changed the title chore: apply kbqInjectElementRef() util fix: added types for elementRef (#DS-4221) Sep 26, 2025
@artembelik artembelik added the bug Something isn't working label Sep 26, 2025
@artembelik artembelik force-pushed the chore/element-ref-util branch from ad588a3 to a7393f5 Compare September 26, 2025 10:19
@artembelik artembelik requested a review from Copilot September 26, 2025 10:19
Copy link

@Copilot 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

Copilot reviewed 90 out of 90 changed files in this pull request and generated 4 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@artembelik artembelik marked this pull request as ready for review September 26, 2025 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants