Skip to content

Conversation

@spike-rabbit
Copy link
Member

@spike-rabbit spike-rabbit commented Oct 31, 2025

What kind of change does this PR introduce? (check one with "x")

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior? (You can also link to an open issue here)

Only decorator based input / outputs on public APIs.

What is the new behavior?

All inputs on the DatatableComponent are now signals that are not reassigned and not getter / setter.

Does this PR introduce a breaking change? (check one with "x")

  • Yes
  • No

If this PR contains a breaking change, please describe the impact and migration path for existing applications:

Be aware that this note also covers future changes. It is just announced once here to avoid overloading the changelog.

Migration to Angular signal inputs/outputs.

All components now use Angular’s input(), model(), and output() functions instead of the @Input/@Output decorators. Consider the following implications for your code:

Template bindings: No changes required! Continue using [inputName] and (outputName) in templates as before.

Programmatic access: Use template binding whenever possible. If not possible, understand that setting input values programmatically is only possible for model() inputs. Reading input values programmatically also requires adjustments for the signal API:

class Component {
  @ViewChild(DatatableComponent) private datatableComponent!: DatatableComponent;
  myFunction(): void {
    // Before
    const value = this.datatableComponent.rows;
    // After
    const value = this.datatableComponent.rows();
  }
}

If programmatically subscribing to outputs is needed, read the following guide: https://angular.dev/guide/components/outputs#subscribing-to-outputs-programmatically

Other information:

We cannot do a release until the migration is completed.

@spike-rabbit spike-rabbit requested a review from a team as a code owner October 31, 2025 11:26
@spike-rabbit spike-rabbit added the breaking-changes Marks issues and PRs that are breaking the API label Oct 31, 2025
@spike-rabbit spike-rabbit force-pushed the refactor/migrate-simple-inputs-to-signals branch 2 times, most recently from b7a38ca to 9ac8bb1 Compare October 31, 2025 11:53
@fh1ch fh1ch requested a review from Copilot October 31, 2025 17:51
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 migrates several @Input() decorators to Angular's modern input() signal API in the datatable component, converting approximately 30 input properties from traditional decorator-based inputs to signal-based inputs. This migration supports Angular's new signal-based reactivity system.

Key changes:

  • Converted multiple @Input() properties to readonly signal inputs using the input() function
  • Updated all usages of these properties throughout the component to call them as functions (e.g., this.scrollbarV() instead of this.scrollbarV)
  • Updated template bindings to call signal inputs as functions
  • Fixed a spelling error in a comment from "rowDraggble" to "rowDraggable"

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
projects/ngx-datatable/src/lib/components/datatable.component.ts Converted 30+ properties from @Input() decorators to signal-based input() calls and updated all references to use function call syntax
projects/ngx-datatable/src/lib/components/datatable.component.html Updated template bindings to call signal inputs as functions by adding () to property references

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

Copy link
Member

@fh1ch fh1ch left a comment

Choose a reason for hiding this comment

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

@spike-rabbit nice one, thanks a lot 🙇

Just one finding, otherwise this one is good to go.

This migrates all inputs to signals
that are not reassigned
and not getter / setter.

The breaking change note also refers to future breaking changes,
so we do not repeat this for every single commit:

BREAKING CHANGE:
Migration to Angular signal inputs/outputs.

All components now use Angular’s `input()`, `model()`, and `output()` functions
instead of the `@Input`/`@Output` decorators. Consider the following
implications for your code:

Template bindings: No changes required! Continue using [inputName] and
(outputName) in templates as before.

Programmatic access: Use template binding whenever possible. If not
possible, understand that setting input values programmatically is only
possible for model() inputs. Reading input values programmatically also
requires adjustments for the signal API:
```ts
class Component {
  @ViewChild(DatatableComponent) private datatableComponent!: DatatableComponent;
  myFunction(): void {
    // Before
    const value = this.datatableComponent.rows;
    // After
    const value = this.datatableComponent.rows();
  }
}
```

If programmatically subscribing to outputs is needed, read the following guide:
https://angular.dev/guide/components/outputs#subscribing-to-outputs-programmatically
@spike-rabbit spike-rabbit force-pushed the refactor/migrate-simple-inputs-to-signals branch from 9ac8bb1 to baa1bab Compare November 3, 2025 08:17
@spike-rabbit spike-rabbit requested a review from fh1ch November 3, 2025 08:18
Copy link
Member

@fh1ch fh1ch left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@fh1ch fh1ch merged commit 4bfbf62 into main Nov 3, 2025
3 checks passed
@fh1ch fh1ch deleted the refactor/migrate-simple-inputs-to-signals branch November 3, 2025 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-changes Marks issues and PRs that are breaking the API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants