tannerntannern / ts-mixer

A small TypeScript library that provides tolerable Mixin functionality.
MIT License
379 stars 27 forks source link

Give init function a default name #44

Closed yaquawa closed 2 years ago

yaquawa commented 2 years ago

The timing of mix decorators be called depend on file name. So I have no idea how to set a default value for the settings in my bootstrap code. To make the settings work, I have to set the settings in every single file where I use the mix decorator. To workaround this, at least we should have a default value for it, or passing a settings object to the decorator itself.

tannerntannern commented 2 years ago

Thanks for the contribution @yaquawa. Unfortunately, I don't think I agree with the suggested changes. Here's my reasoning:

we should have a default value for it

The special initFunction handling should be opt-in behavior, not enabled by default. It's possible that someone using ts-mixer could happen to have an init function in their class without intending it to be called automatically. For example, they could be using a builder pattern with a fluent interface.

or passing a settings object to the decorator itself

I'm more inclined to go with something like this, but it has a number of challenges as well. For starters, I'm not seeing a good way to define the signature of @mix with an optional settings argument (typing/intellisense-wise), because TypeScript doesn't allow any arguments to follow a rest parameter (e.g., function mix(...classes: Class[], settings?: Settings) {...).

But even if there is a good way to define the signature, I'm willing to bet that most users don't want to customize the initFunction differently for every @mix call, but will rather have a consistent, application-wide setting that they want to use (which appears to be your situation as well?) This makes the value of being able to pass a settings object to the @mix decorator questionable.

I have two possible suggestions for you:

  1. Configure the ts-mixer settings in a file that you know will be loaded before all others. Many applications have a single entry-point to the rest of the code (e.g., index.ts) so you could guarantee that your settings will be set properly this way.
  2. If your application doesn't have a single entry-point, you could define a wrapper function for the @mix decorator that reapplies the settings you need before each invocation. Then you could use your wrapper function instead of @mix (this should have virtually no performance impact). For example:
    
    import { mix as tsMix, settings } from 'ts-mixer';

export const mix: typeof tsMix = (...classes) => { settings.initFunction= 'init'; return tsMix(...classes); }



Let me know if either of these solutions will work for you, or if you have other arguments to make in favor of having a default `initFunction`.
yaquawa commented 2 years ago

@tannerntannern The wrapper solution seems good for me, thanks for the sample code, it helps!

It's possible that someone using ts-mixer could happen to have an init function in their class without intending it to be called automatically.

I think we can have the default "init" function name as symbol but not string, just like this:

import { init } from 'ts-mixer';

class Foo {
  [init](...args){
    ...
  }
}
tannerntannern commented 2 years ago

@yaquawa Glad the wrapper solution works for you!

I also love your suggestion to make the init function name a symbol! I generally consider ts-mixer to be in maintenance mode so I don't take time to add new features unless they are critical, but I would gladly accept a PR if you wanted to give it a crack! I would just take care to make it backwards-compatible so I don't have to do another major version bump.

I will close this PR for now since if the init symbol change is made, it will be a separate PR