ng-dynamic-component icon indicating copy to clipboard operation
ng-dynamic-component copied to clipboard

ndcDynamicOutputs: thisArg of handler

Open ajafff opened this issue 5 years ago • 4 comments

Angular Version: 11.0.9 ng-dynamic-component: 8.0.0

https://stackblitz.com/edit/angular-ivy-jdgusx?file=src/app/app.component.html

The handler of ndcDynamicOutputs is called without a thisArg. In the example above that means clicking the button calls onClick without a this reference and this.clicked inside fails with an exception. This was surprising to me.

I know that I can declare the handler method as arrow function. But there are some cases where I cannot do that, e.g. inheritance.

I can think of 2 possible solutions:

  • Automatically use the host component as thisArg. I don't know if that's possible without accessing private Angular functions.
  • allow {handler: onClick, thisArg: this} and use the provided thisArg when calling handler in IoService

ajafff avatar Jan 14 '21 09:01 ajafff

Hi @ajafff, thanks for reporting this. I am well aware of it and in fact already started a work in progress to address context issue in a branch https://github.com/gund/ng-dynamic-component/blob/8d4459314d29d9c530e030db97f007e5c79c3fe3/projects/ng-dynamic-component/src/lib/io/io.service.ts#L344-L346

Basically the idea is to use DI token to provide a context for outputs:

  • There is a static (global) DI Token that can be used to provide context straight from component's providers via UseExisting provider
  • And there is a dynamic DI Token that must resolve to a provider and it is created and injected in the IO service which allows you to setup your own global strategy how context is provided at the app level https://github.com/gund/ng-dynamic-component/blob/8d4459314d29d9c530e030db97f007e5c79c3fe3/projects/ng-dynamic-component/src/lib/io/io.service.ts#L316-L328

I was also doing some cleanups along the way so it's not ready yet but context already works so if you want you can give it a try locally and/or welcome to give any feedback =)

gund avatar Jan 15 '21 11:01 gund

Also I did not know that you can use this in Angular template and it will actually work, so maybe I can add an option to provide context straight from template like [ouptuts]="{ output: { handler: handlerFn, context: this } }". The name though must be context to be in line with the rest of context DI providers functionality.

gund avatar Jan 15 '21 11:01 gund

If you don't mind tapping into undocumented Angular internals, you don't need all the injection tokens and instead look up the context if not explicitly provided by the user. Here's how you get the context from a directive:

class MyDirective {
  constructor(@SkipSelf() public cd: ChangeDetectorRef) {}

  getContext() {
    if (this["__ngContext__"]) {
      // Ivy
      return this["__ngContext__"][16][8];
    } else {
      // ViewEngine
      return this.cd["context"];
    }
  }
}

I wrote this little test app, that prints to the console, whether the determined context matches the actual context (which is passed via Input binding). I added all kinds of nesting and transplanted views I could think of. If you look at the console you can see it logs true for all cases. https://stackblitz.com/edit/angular-ivy-366dmj?file=src/app/app.component.ts

ajafff avatar Jan 16 '21 14:01 ajafff

This is the reason I do not want to built it in the lib - because if this implementation detail changes at any point (even between minor updates) it will break apps and will require new patch releases.

That's why I want to provide extension point to implement context strategy by users directly so they have full control over it.

And this approach can be easily implemented globally on app level by providing IoEventContextProviderToken like so (not yet tested though):

import { NgModule, Injector, StaticProvider, ChangeDetectorRef, InjectFlags } from '@angular/core';
import { DynamicIoDirective, IoEventContextToken, IoEventContextProviderToken } from 'ng-dynamic-component';

export function resolveComponentContext(injector: Injector) {
  const ioDirective = injector.get(DynamicIoDirective);

  // Ivy
  if (ioDirective["__ngContext__"]) {
    return ioDirective["__ngContext__"][16][8];
  }

  // ViewEngine
  const cdr = injector.get(ChangeDetectorRef, undefined, InjectFlags.SkipSelf);
  return cdr["context"];
}

const componentContextProvider: StaticProvider = {
  provide: IoEventContextToken,
  useFactory: resolveComponentContext,
  deps: [Injector]
};

@NgModule({
  providers: [
    { provide: IoEventContextProviderToken, useValue: componentContextProvider }
  ]
})
export class AppModule {}

gund avatar Jan 16 '21 16:01 gund

Since version v10.4.0 it's now possible to set the context for output handlers. I'm closing this issue as it's outdated. If you want to reopen the discussion please feel free to open a new issue.

gund avatar Aug 28 '22 01:08 gund