scullyio / scully

The Static Site Generator for Angular apps
https://scully.io/
MIT License
2.56k stars 257 forks source link

Add decorator to provide TransferState functionality #1191

Open chriskinzel opened 3 years ago

chriskinzel commented 3 years ago

🧩 Feature request

Description

Currently, to transfer state from Scully to a running app, we must inject and wrap values with TransferStateService like so

e.g.

export class MyComponent {
  public readonly someData$: Observable<string> = this.transferStateService.useScullyTransferState(
    `${MyComponent.name}_someData$`,
    this.http.get('https://somedata.com')
  );

  constructor(
    private readonly http: HttpClient,
    private readonly transferStateService: TransferStateService
  ) {}
}

it would be nice if it were possible to do this using a decorator instead.

Describe the solution you'd like

Example using decorator:

export class MyComponent {
  @TransferState
  public readonly someData$: Observable<string> = this.http.get('https://somedata.com');

  constructor(private readonly http: HttpClient) {}
}
SanderElias commented 3 years ago

@chriskinzel I like the idea. There is an inherent issue here. To be able to do this, I need the transferState service injected into the decorator. As decorators work outside of Angular's scope this is a bit problematic. We really need access to the app's injector, because the transferState service is depending on Angulars router.

chriskinzel commented 3 years ago

Yeah I was thinking about how this could be done, I think one way to do it is to have an interface that must be implemented on the component to use this feature. It would require a property transferStateService: TransferStateService to be present (user would have to inject this manually, don't see how it could be done automatically with just a decorator). Then the decorator could just lookup this field on the component object to access to the service and wrap the target observable. The decorator would need to defer this operation until a component instance is instantiated.

SanderElias commented 3 years ago

@chriskinzel Hmm, When there is a mandatory need to inject the service and hook it up to the class, most of the simplicity of the decorator would be gone? Do you really think the gain id big enough in that case? I'm going to mull this one over for a while, I don't like decorators because they are a TS and not a JS thing, but this would make the adaption of theTS a bit easier.

chriskinzel commented 3 years ago

@SanderElias I agree the gain is a bit marginal but I think it still has an impact in reducing boilerplate associated with wrapping observables with this.stateTransferService.useScullyTransferState(...). Not only is something like @TransferState less typing, the decorator could also auto-generate the key using the class name and the property key which would be nice.

SanderElias commented 3 years ago

That's true. I'll spike this, and see if I can get a hold of the injector inside a decorator without actually adding anything to the class. Otherwise, this will be a support night mare.