Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(angular-table): Refactor Flex render implementation - Zoneless, Better type safety, allows reactive values into cell content, re-render when cell context changes, allow to pass signal inputs into custom components #5856

Open
wants to merge 26 commits into
base: main
Choose a base branch
from

Conversation

riccardoperra
Copy link
Contributor

@riccardoperra riccardoperra commented Jan 2, 2025

I refactored how the FlexRender directive applies updates internally during change detection. This will resolve issues like #5767 without necessarily waiting for v9 (so this is not a breaking change for users). It now should also work with zoneless and it's reactive if someone use signals into the cell content. Also there are some dx improvements that helps writing column custom content easily

tldr

For those who enjoy reading 😅:

Update vitest configuration

I've updated vitest test setup with latest version of @analog/vitest-angular and @analog/vite-plugin-angular in order to compile the angular components correctly.

We are now able to use angular signal inputs during test. I removed all workaround utils we had to use before. Now we could also opt for switching the FlexRenderDirective to signal inputs instead of old-based @input decorators, even if we cannot rely on effects, we should always use ngOnChanges).

Enhanced Type Safety for FlexRenderComponent

The FlexRenderComponent inputs are now typed based on the given component instance

@Component()
class MyComp {   
  readonly input1 = input<string>();
  readonly input2 = input.required<number>();
}

new FlexRenderComponent(MyComp, { ... }) // correctly typed as { input1: string | undefined, input2: number }

I have also added a flexRenderComponent function util that can be used to enhance typesafety and which use an option object instead of multiple arguments to pass data. This allows us to add new arguments in the future without breaking that signature (e.g. in the future we will probably need to bind angular "outputs").

In v9 we may want to have only one of the two available to avoid confusion?

@Component()
class Example {   
  readonly input1 = input<string>();
}

//  Example doesn't have required inputs, so 2nd argument is optional. `input1` is typed as `string | undefined`
flexRenderComponent(Example)

@Component()
class ExampleRequired {   
  readonly input1 = input.required<string>();
}

//  Example doesn't have required inputs, so 2nd argument is mandatory. `input1` is typed as `string`
flexRenderComponent(ExampleRequired, { inputs: { input1: "value" } )

Injection context for cell content

Cell content now runs within an injection context. This allows users to inject services and using signals into the cell definition, even outside angular components.

// file outside angular
const columns = [ 
  {
      accessorFn: row => row.lastName,
      id: 'lastName',
      header: () => {
        return inject(MyService).returnSomething(...)
      }
  }
]

Refactor FlexRenderDirective rendering mechanism

tldr

This is the biggest part. With the previous implementation, we were only relying on ngOnChanges hook.

ngOnChanges is called by Angular when any data-bound property change, but it's not enough for some reasons:

  • content property is a reference of the cell/header definition, which could be a function. Doing content(props) may return different values, but since we are passing a reference, Angular cannot evaluate it for us automatically.
  • props most of the time will be (cell|header).getContext(), which is memoized for cell (but no headers). Then even in this case the reference may not change

When table data change, we are sure that at least both properties changes. Currently this is the only case were we are re-rendering the cell view.

With this PR, first I am introducing some logic to split some rendering-related code on separate classes (those are not exposed to the library consumers):

  • FlexRenderComponentRef: a wrapper for Angular ComponentRef, which expose some internal utils to detect changes, set inputs via angular setInput api etc.
  • FlexRenderComponentFactory: an angular service that is responsible to create instances of FlexRenderComponentRef.
  • FlexRenderView: an abstract class that encapsulate the view state and some information, and defines some logic that we invoke in specific phases of the view checker. It has two implementations:
    • FlexRenderTemplateView: a class that define update methods for content rendered via EmbeddedViewRef (which we use to render primitives like string, number or even TemplateRef instances)
    • FlexRenderComponentView: a class that define update methods for content rendered via FlexRenderComponentRef (ComponentRef)
  • Code related to FlexRenderComponent injectable context has been moved into a separate file (those declarations are exported for library consumers, but they have not changed)

I've then revisited the rendering business logic via ngDoCheck hook and implementing Bit fields to handle the rendering phases.

Link: angular-table/src/flex-render/flags.ts

/**
 * Flags used to manage and optimize the rendering lifecycle of content of the cell,
 * while using FlexRenderDirective.
 */
export enum FlexRenderFlags {
  /**
   * Indicates that the view is being created for the first time or will be cleared during the next update phase.
   * This is the initial state and will transition after the first ngDoCheck.
   */
  ViewFirstRender = 1 << 0,
  /**
   * Represents a state where the view is not dirty, meaning no changes require rendering updates.
   */
  Pristine = 1 << 1,
  /**
   * Indicates the `content` property has been modified or the view requires a complete re-render.
   * When this flag is enabled, the view will be cleared and recreated from scratch.
   */
  ContentChanged = 1 << 2,
  /**
   * Indicates that the `props` property reference has changed.
   * When this flag is enabled, the view context is updated based on the type of the content.
   *
   * For Component view, inputs will be updated and view will be marked as dirty.
   * For TemplateRef and primitive values, view will be marked as dirty
   */
  PropsReferenceChanged = 1 << 3,
  /**
   * Indicates that the current rendered view needs to be checked for changes.
   */
  DirtyCheck = 1 << 4,
  /**
   * Indicates that a signal within the `content(props)` result has changed
   */
  DirtySignal = 1 << 5,
  /**
   * Indicates that the first render effect has been checked at least one time.
   */
  RenderEffectChecked = 1 << 6,
}

Regardless of the implementation, there are two new behaviors:

Initializing an effect during the first render

During the view first render, we initialize an effect that set the DirtySignal flag every time the content(props) changes, if signals are used into the cell definition. This allows to react to changes even without ngZone, and opens up some possibilities in v9 to further improve rendering, for example detecting also when the table state signals changes

Checking changes in ngDoCheck

During the ngDoCheck lifecycle hook, we determine whether the view needs to be updated by evaluating a set of flags and comparing the result of content(props) with its previous value. At the moment, this check occurs during every doCheck cycle. However, in a future we could use store slices (v10?+) and signals (v9 with the reactivity feature i was working on) to improve this phase updating more efficently.


What changes for users

The new implementation is retrocompatible with previous versions, but improves dx of columns definition while using Angular since this let now to render content conditionally, using signals inside etc.

Most of the time the code I saw to render custom content was something like this, splitting how the table is rendered in both template and typescript

<tbody>
  @for (row of table.getRowModel().rows; track row.id) {
    <tr>
      @for (cell of row.getVisibleCells(); track cell.id) {
        <td>
          @switch (cell.id) {
            <!-- this could have be done also in column def, but what if we want to add some custom logic? -->
            @case ('user') {
              {{ cell.row.original.firstName }}
              {{ cell.row.original.lastName }}
            }
            @case ('status') {
              @if (colors().length) { 
                <app-colored-badge [colors]="colors()" [status]="cell.row.original.status"></app-colored-badge>
              } @else {
                 {{ 'status'. + cell.row.original.status | translate }}
              }
            }
           <!-- This is for columns that can just use the `renderValue` :/ -->
            @default {
              <ng-container
                *flexRender="
                  cell.column.columnDef.cell;
                  props: cell.getContext();
                  let cell
                "
              >
                <div [innerHTML]="cell"></div>
              </ng-container>
            }
          }
        </td>
      }
    </tr>
  }
</tbody>

Now, we can do the same directly into the column definitions 😄

const helper = createColumnHelper<Person>()
const columns = [
  helper.display({
    id: 'user',
    cell: context => {
      const { firstName, lastName } = context.row.original
      // Just an example of a service that may concat non-empty string
      const stringHelper = inject(StringHelper)
      return stringHelper.joinNonEmpty([firstName, lastName], ' ')

      // or maybe i want to render a component in some specific conditions
      const { firstName, lastName, email } = context.row.original
      if (!email) { 
        return `${firstName} ${lastName}`
      }
      return new FlexRenderComponent(
        CustomCell, 
        { content: `${firstName} ${lastName}`, tooltip: email }
      )
    },
  }),
  helper.accessor('status', {
    id: 'status',
    cell: ({ getValue }) => {
      const status = getValue()
      // this may be a signal, then everything here will be revaluated also when its value change.
      const colors = appColors()
      if (!colors) {
        // just an example
        return translate(`status.${status}`)
      }
      return new FlexRenderComponent(AppColoredBadge, {
        status,
        colors,
      })
    },
  }),
]

Copy link

nx-cloud bot commented Jan 2, 2025

View your CI Pipeline Execution ↗ for commit 1d9b642.

Command Status Duration Result
nx affected --targets=test:format,test:sherif,t... ✅ Succeeded 1m 27s View ↗
nx run-many --targets=build --exclude=examples/** ✅ Succeeded 35s View ↗

☁️ Nx Cloud last updated this comment at 2025-01-09 21:47:40 UTC

@KevinVandy
Copy link
Member

I think we need to do something to fix this:

image

We have the typescript version pinned, but it's still installing the wrong version in some peer deps somehow.

cc @riccardoperra @lachlancollins

@riccardoperra riccardoperra force-pushed the feat/angular-flex-render-granular-update branch from 7bfea75 to 94bed11 Compare January 3, 2025 08:26
Copy link

pkg-pr-new bot commented Jan 3, 2025

Open in Stackblitz

More templates

@tanstack/angular-table

npm i https://pkg.pr.new/@tanstack/angular-table@5856

@tanstack/qwik-table

npm i https://pkg.pr.new/@tanstack/qwik-table@5856

@tanstack/lit-table

npm i https://pkg.pr.new/@tanstack/lit-table@5856

@tanstack/react-table

npm i https://pkg.pr.new/@tanstack/react-table@5856

@tanstack/react-table-devtools

npm i https://pkg.pr.new/@tanstack/react-table-devtools@5856

@tanstack/match-sorter-utils

npm i https://pkg.pr.new/@tanstack/match-sorter-utils@5856

@tanstack/solid-table

npm i https://pkg.pr.new/@tanstack/solid-table@5856

@tanstack/svelte-table

npm i https://pkg.pr.new/@tanstack/svelte-table@5856

@tanstack/table-core

npm i https://pkg.pr.new/@tanstack/table-core@5856

@tanstack/vue-table

npm i https://pkg.pr.new/@tanstack/vue-table@5856

commit: 1d9b642

@riccardoperra riccardoperra force-pushed the feat/angular-flex-render-granular-update branch from 144f0b7 to ed5c89c Compare January 5, 2025 14:31
@riccardoperra riccardoperra force-pushed the feat/angular-flex-render-granular-update branch from ed5c89c to db39511 Compare January 5, 2025 14:32
riccardoperra and others added 3 commits January 5, 2025 16:14
test
fix doc
add flexRenderComponent util
@riccardoperra riccardoperra force-pushed the feat/angular-flex-render-granular-update branch from 5753653 to 2c1725b Compare January 5, 2025 16:58
@riccardoperra riccardoperra marked this pull request as ready for review January 5, 2025 16:58
@riccardoperra riccardoperra marked this pull request as draft January 5, 2025 18:30
@riccardoperra riccardoperra marked this pull request as ready for review January 5, 2025 18:56
@riccardoperra
Copy link
Contributor Author

riccardoperra commented Jan 6, 2025

@KevinVandy pr is now ready. I've also added a fix to proxy that was preventing to have getRowModel reactive in some cases.

4f0149a#diff-a3500075d578d363191c8bc27e94c7f42a2426e4b6e5c582551cfafe9d2576b4R27

3b2f6fa#diff-31817a6cdcff3b5737ed13d14709201ade91cc98fd8524fd8898e0afc5ac9415R88

After that I'll have another ready PR to support angular outputs while using flexRender but I would prefer to separate it to make it shorter (example of usage)

@riccardoperra riccardoperra changed the title feat(angular-table): Refactor and improve flex render implementation to update component content during change detection feat(angular-table): Refactor Flex render implementation - Zoneless, Better type safety, allows reactive values into cell content, re-render when cell context changes, allow to pass reactive inputs into components Jan 6, 2025
@riccardoperra riccardoperra changed the title feat(angular-table): Refactor Flex render implementation - Zoneless, Better type safety, allows reactive values into cell content, re-render when cell context changes, allow to pass reactive inputs into components feat(angular-table): Refactor Flex render implementation - Zoneless, Better type safety, allows reactive values into cell content, re-render when cell context changes, allow to pass signal inputs into custom components Jan 6, 2025
@riccardoperra riccardoperra force-pushed the feat/angular-flex-render-granular-update branch from fbd4df0 to f822645 Compare January 6, 2025 11:39
…port-output-binding

add support for angular outputs in flex-render-component
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.

3 participants