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): improvements to angularReactivityFeature, try to remove proxies #5921

Draft
wants to merge 13 commits into
base: alpha
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/angular-table/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
"test:build": "publint --strict",
"test:eslint": "eslint ./src",
"test:lib": "vitest",
"test:benchmark": "vitest bench",
"test:lib:dev": "vitest --watch",
"test:types": "tsc && vitest --typecheck"
},
Expand Down
146 changes: 96 additions & 50 deletions packages/angular-table/src/angularReactivityFeature.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { computed, signal } from '@angular/core'
import { toComputed } from './proxy'
import { computed, isSignal, signal } from '@angular/core'
import { defineLazyComputedProperty, markReactive } from './reactivityUtils'
import type { Signal } from '@angular/core'
import type {
RowData,
Expand All @@ -20,16 +20,26 @@ declare module '@tanstack/table-core' {
> extends Table_AngularReactivity<TFeatures, TData> {}
}

type SkipPropertyFn = (property: string) => boolean

export interface AngularReactivityFlags {
header: boolean | SkipPropertyFn
column: boolean | SkipPropertyFn
row: boolean | SkipPropertyFn
cell: boolean | SkipPropertyFn
table: boolean | SkipPropertyFn
}

interface TableOptions_AngularReactivity {
enableExperimentalReactivity?: boolean
reactivity?: Partial<AngularReactivityFlags>
}

interface Table_AngularReactivity<
TFeatures extends TableFeatures,
TData extends RowData,
> {
_rootNotifier?: Signal<Table<TFeatures, TData>>
_setRootNotifier?: (signal: Signal<Table<TFeatures, TData>>) => void
get: Signal<Table<TFeatures, TData>>
_setTableNotifier: (signal: Signal<Table<TFeatures, TData>>) => void
}

interface AngularReactivityFeatureConstructors<
Expand All @@ -40,78 +50,104 @@ interface AngularReactivityFeatureConstructors<
Table: Table_AngularReactivity<TFeatures, TData>
}

const getUserSkipPropertyFn = (
value: undefined | null | boolean | SkipPropertyFn,
defaultPropertyFn: SkipPropertyFn,
) => {
if (typeof value === 'boolean') {
return defaultPropertyFn
}
return value ?? defaultPropertyFn
}

export function constructAngularReactivityFeature<
TFeatures extends TableFeatures,
TData extends RowData,
>(): TableFeature<AngularReactivityFeatureConstructors<TFeatures, TData>> {
return {
getDefaultTableOptions(table) {
return { enableExperimentalReactivity: false }
return {
reactivity: {
header: true,
column: true,
row: true,
cell: true,
table: true,
},
}
},
constructTableAPIs: (table) => {
if (!table.options.enableExperimentalReactivity) {
return
}
const rootNotifier = signal<Signal<any> | null>(null)

table._rootNotifier = computed(() => rootNotifier()?.(), {
equal: () => false,
}) as any

table._setRootNotifier = (notifier) => {
table._setTableNotifier = (notifier) => {
rootNotifier.set(notifier)
}

setReactiveProps(table._rootNotifier!, table, {
skipProperty: skipBaseProperties,
table.get = computed(() => rootNotifier()!(), {
equal: () => false,
})

if (table.options.reactivity?.table === false) {
return
}
markReactive(table)
setReactiveProps(table.get, table, {
skipProperty: getUserSkipPropertyFn(
table.options.reactivity?.table,
skipBaseProperties,
),
})
},

constructCellAPIs(cell) {
if (
!cell._table.options.enableExperimentalReactivity ||
!cell._table._rootNotifier
) {
if (cell._table.options.reactivity?.cell === false) {
return
}
setReactiveProps(cell._table._rootNotifier, cell, {
skipProperty: skipBaseProperties,
markReactive(cell)
setReactiveProps(cell._table.get, cell, {
skipProperty: getUserSkipPropertyFn(
cell._table.options.reactivity?.cell,
skipBaseProperties,
),
})
},

constructColumnAPIs(column) {
if (
!column._table.options.enableExperimentalReactivity ||
!column._table._rootNotifier
) {
if (column._table.options.reactivity?.column === false) {
return
}
setReactiveProps(column._table._rootNotifier, column, {
skipProperty: skipBaseProperties,
markReactive(column)
setReactiveProps(column._table.get, column, {
skipProperty: getUserSkipPropertyFn(
column._table.options.reactivity?.cell,
skipBaseProperties,
),
})
},

constructHeaderAPIs(header) {
if (
!header._table.options.enableExperimentalReactivity ||
!header._table._rootNotifier
) {
if (header._table.options.reactivity?.header === false) {
return
}
setReactiveProps(header._table._rootNotifier, header, {
skipProperty: skipBaseProperties,
markReactive(header)
setReactiveProps(header._table.get, header, {
skipProperty: getUserSkipPropertyFn(
header._table.options.reactivity?.cell,
skipBaseProperties,
),
})
},

constructRowAPIs(row) {
if (
!row._table.options.enableExperimentalReactivity ||
!row._table._rootNotifier
) {
if (row._table.options.reactivity?.row === false) {
return
}
setReactiveProps(row._table._rootNotifier, row, {
skipProperty: skipBaseProperties,
markReactive(row)
setReactiveProps(row._table.get, row, {
skipProperty: getUserSkipPropertyFn(
row._table.options.reactivity?.cell,
skipBaseProperties,
),
})
},
}
Expand All @@ -120,10 +156,19 @@ export function constructAngularReactivityFeature<
export const angularReactivityFeature = constructAngularReactivityFeature()

function skipBaseProperties(property: string): boolean {
return property.endsWith('Handler') || !property.startsWith('get')
return (
// equals `getContext`
property === 'getContext' ||
// start with `_`
property[0] === '_' ||
// doesn't start with `get`, but faster
!(property[0] === 'g' && property[1] === 'e' && property[2] === 't') ||
// ends with `Handler`
property.endsWith('Handler')
)
}

export function setReactiveProps(
function setReactiveProps(
notifier: Signal<Table<any, any>>,
obj: { [key: string]: any },
options: {
Expand All @@ -133,16 +178,17 @@ export function setReactiveProps(
const { skipProperty } = options
for (const property in obj) {
const value = obj[property]
if (typeof value !== 'function') {
continue
}
if (skipProperty(property)) {
if (
isSignal(value) ||
typeof value !== 'function' ||
skipProperty(property)
) {
continue
}
Object.defineProperty(obj, property, {
enumerable: true,
configurable: false,
value: toComputed(notifier, value, property),
defineLazyComputedProperty(notifier, {
valueFn: value,
property,
originalObject: obj,
})
}
}
67 changes: 58 additions & 9 deletions packages/angular-table/src/flex-render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,34 +2,41 @@ import {
ChangeDetectorRef,
Directive,
DoCheck,
effect,
type EffectRef,
Inject,
inject,
Injector,
Input,
OnChanges,
runInInjectionContext,
SimpleChanges,
TemplateRef,
Type,
ViewContainerRef,
effect,
inject,
runInInjectionContext,
} from '@angular/core'
import { memo } from '@tanstack/table-core'
import { FlexRenderComponentProps } from './flex-render/context'
import { FlexRenderFlags } from './flex-render/flags'
import {
flexRenderComponent,
FlexRenderComponent,
flexRenderComponent,
} from './flex-render/flex-render-component'
import { FlexRenderComponentFactory } from './flex-render/flex-render-component-ref'
import {
FlexRenderComponentView,
FlexRenderTemplateView,
type FlexRenderTypedContent,
FlexRenderView,
mapToFlexRenderTypedContent,
} from './flex-render/view'
import { memo } from '@tanstack/table-core'
import type { FlexRenderTypedContent } from './flex-render/view'
import type {
CellContext,
HeaderContext,
Table,
TableFeatures,
} from '@tanstack/table-core'
import type { EffectRef } from '@angular/core'
import { isReactive } from './reactivityUtils'

export {
injectFlexRenderContext,
Expand All @@ -51,7 +58,12 @@ export type FlexRenderContent<TProps extends NonNullable<unknown>> =
standalone: true,
providers: [FlexRenderComponentFactory],
})
export class FlexRenderDirective<TProps extends NonNullable<unknown>>
export class FlexRenderDirective<
TProps extends
| NonNullable<unknown>
| CellContext<TableFeatures, any>
| HeaderContext<TableFeatures, any>,
>
implements OnChanges, DoCheck
{
readonly #flexRenderComponentFactory = inject(FlexRenderComponentFactory)
Expand All @@ -68,9 +80,13 @@ export class FlexRenderDirective<TProps extends NonNullable<unknown>>
@Input({ required: true, alias: 'flexRenderProps' })
props: TProps = {} as TProps

@Input({ required: false, alias: 'flexRenderNotifier' })
notifier: 'doCheck' | 'tableChange' = 'doCheck'

@Input({ required: false, alias: 'flexRenderInjector' })
injector: Injector = inject(Injector)

table: Table<TableFeatures, any>
renderFlags = FlexRenderFlags.ViewFirstRender
renderView: FlexRenderView<any> | null = null

Expand All @@ -97,7 +113,9 @@ export class FlexRenderDirective<TProps extends NonNullable<unknown>>

ngOnChanges(changes: SimpleChanges) {
if (changes['props']) {
this.table = 'table' in this.props ? this.props.table : null
this.renderFlags |= FlexRenderFlags.PropsReferenceChanged
this.bindTableDirtyCheck()
}
if (changes['content']) {
this.renderFlags |=
Expand All @@ -114,8 +132,13 @@ export class FlexRenderDirective<TProps extends NonNullable<unknown>>
return
}

this.renderFlags |= FlexRenderFlags.DirtyCheck
if (this.notifier === 'doCheck') {
this.renderFlags |= FlexRenderFlags.DirtyCheck
this.doCheck()
}
}

private doCheck() {
const latestContent = this.#getContentValue()
if (latestContent.kind === 'null' || !this.renderView) {
this.renderFlags |= FlexRenderFlags.ContentChanged
Expand All @@ -129,6 +152,32 @@ export class FlexRenderDirective<TProps extends NonNullable<unknown>>
this.update()
}

#tableChangeEffect: EffectRef | null = null

private bindTableDirtyCheck() {
this.#tableChangeEffect?.destroy()
this.#tableChangeEffect = null
let firstCheck = !!(this.renderFlags & FlexRenderFlags.ViewFirstRender)
if (
this.table &&
this.notifier === 'tableChange' &&
isReactive(this.table)
) {
this.#tableChangeEffect = effect(
() => {
this.table.get()
if (firstCheck) {
firstCheck = false
return
}
this.renderFlags |= FlexRenderFlags.DirtyCheck
this.doCheck()
},
{ injector: this.injector, forceRoot: true },
)
}
}

update() {
if (
this.renderFlags &
Expand Down
2 changes: 1 addition & 1 deletion packages/angular-table/src/flex-render/context.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { inject, InjectionToken } from '@angular/core'
import { InjectionToken, inject } from '@angular/core'

export const FlexRenderComponentProps = new InjectionToken<
NonNullable<unknown>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ export function flexRenderComponent<
*/
export class FlexRenderComponent<TComponent = any> {
readonly mirror: ComponentMirror<TComponent>
readonly allowedInputNames: string[] = []
readonly allowedOutputNames: string[] = []
readonly allowedInputNames: Array<string> = []
readonly allowedOutputNames: Array<string> = []

constructor(
readonly component: Type<TComponent>,
Expand Down
4 changes: 2 additions & 2 deletions packages/angular-table/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ export * from './angularReactivityFeature'
export * from './createTableHelper'
export * from './flex-render'
export * from './injectTable'
export * from './lazy-signal-initializer'
export * from './proxy'
export * from './lazySignalInitializer'
export * from './reactivityUtils'
export * from './flex-render/flex-render-component'
Loading
Loading