Defining property in constructor vs class member results in different behavior.

[ShawnTalbert]

🐛 Bug Report

How you define a property in TS effects its initial value unexpectedly (perhaps due to default DI behavior)?

Using @jwx's example tutorial as reference auth.ts,

Change the declaration of the auth property from:

export class Auth {
    public user: string;

to a constructor declared property:

export class Auth {
  constructor(public user:string){}
}

🤔 Expected Behavior

After instantiation, I expect this.user to be undefined in both cases, or at least have the same value regardless of TypeScript declaration technique?

😯 Current Behavior

In the first case, user == undefined. In the second case, user == String('') (string object, length zero).

I even tried default assignment in the constructor (constructor(public user:string = void 0)) but that still gets overwritten with a String('').

💁 Possible Solution

I'm guessing this is caused by the default DI behavior trying to resolve the constructor parameter version of user?

🔦 Context

This actually breaks the my-app tutorial view where the if.bind=!auth.user is used. In the plain class field case !auth.user evaluates true. In the constructor declared case it evaluates false (because auth.user is an instance of a String)

💻 Code Sample

@jwx main tutorial repo is here

🌍 Your Environment

Software	Version(s)
Aurelia	[email protected]
Language
Browser
Bundler
Operating System	Linux
NPM/Node/Yarn	npm

[fkleuver]

The default (expected) behavior of DI is indeed to try to resolve each constructor parameter. One could argue that it never makes sense to attempt to resolve primitives like this, so we could add some exclusions, but on the other hand this is not how you're supposed to declare properties that aren't meant to be passed in to the constructor.

So this is in fact the correct way of doing it in your situation:

export class Auth {
    public user: string;

You might think that TS declaration technique shouldn't matter, but it very much does. There are runtime differences in the code that TS generates, so you can expect different behavior regardless of Aurelia. This is by nature of TS. This:

export class Auth {
    public user: string;

Is transpiled to this:

export class Auth {

Or, if you have the new define property flag enabled:

export class Auth {
  constructor() {
    Object.defineProperty(this, 'user', { value: undefined });
  }

Whereas this:

export class Auth {
  constructor(public user:string){}
}

Is transpiled to this:

export class Auth {
  constructor(user) {
    this.user = user; // value comes from a formal parameter now
  }

[ShawnTalbert]

In my (classical OO) experience constructors are ideally meant to provide the consumer an initialized object. In fact, the --strictPropertyInitialization option tries to help ensure that's the case in TS.

Aside from OO philosophy, what is typically expected to have same behavior is something like this: (these two compile to exactly the same JScode):

class Auth {
user:string
constructor(user:string) {
this.user = user
}

and

class Auth {
constructor(public user:string){}
}

and in both cases all properties of the Auth class will be guaranteed initialized by the time the caller gets the instance because the constructor forces it.

Thinking about the particular situation we have, would it be possible perhaps to allow Aurelia to only provide default DI for constructor arguments only if they don't have initialization? In other words, could Aurelia not try and provide a value for user if I've already provided one (initialized it) like so?

class Auth {
 constructor(public user:string = 'please do not DI on top of my default here'){}
}

I would love to be able to use the concise constructor prop declaration syntax in my Aurelia apps, and still somehow have it play nicely with DI. As a workaround, could I manually configure the main container to resolve primitive strings to null/undefined by default?

As another idea, is there a way to annotate my constructor parameter to NOT have a default value injected (@noinject)?

As a final idea, could the default injection for primitives resolve to their natural JS defaults (i.e. undefined)?

[fkleuver]

Of course we offer various ways to override the defaults, but the defaults are how they are because of the decorator metadata emitted by TypeScript. Aurelia's DI utilizes this metadata.

As a workaround, could I manually configure the main container to resolve primitive strings to null/undefined by default?

Just add this to your registrations:

.register(Registration.instance(String, undefined))

As another idea, is there a way to annotate my constructor parameter to NOT have a default value injected (@noinject)?

Sort of. You can fabricate a specific value decorator for example::

export const Undefined = DI.createInterface().withDefault(x => x.instance(undefined));

// ...

export class Auth {
  constructor(@Undefined public user: string) {}
}

As a final idea, could the default injection for primitives resolve to their natural JS defaults (i.e. undefined)?

I'm on the fence about that. Why just primitives? What about other built-ins like Function, Object, Promise, etc? We could exclude all ES built-in types, and there would still be cases where it fails such as cross-realm calls. There are ways around that too, but those would be pretty bad for performance. So doing this robustly would be a perf hazard and a lot of extra work, and I prefer not doing things only half-ish.

I respect your preference for the inline declaration for cosmetic purposes but from a technical point of view when you declare a constructor parameter, you declare an expectation for the caller to provide a value for that parameter. I'm inclined to say just do this, which makes much more sense to me:

export class Auth {
  public user: string | undefined = undefined;
}

[ShawnTalbert]

Thanks @fkleuver. I guess I'll have to go with your last example (explicitly including undefined on the type) and give up on treating the constructor as the single point of initialization for an instance.

I agree declaring a constructor parameter does/should put the burden on the caller. The catch here is the caller is an automaton named 'DI'. If it were a human caller, I'd expect that human to provide the value of their choice for user.

As a point of comparison, I looked this up in Angular and it seems their general approach is the same - all constructor arguments are meant to be supplied through DI. There are ways around it but it's klunky IMHO.

It would have been nice to have a low/no overhead mechanism to just ignore marked constructor params but it looks like that's not happening.

This may be common in modern web-dev but it will take me some time to get used to the idea that I can't control the initialization of my class via the constructor. Some properties (well, all that aren't mentioned in the constructor) will be left undefined even after instantiation. That makes me cry a little bit. I guess I'll cozy up to the ?? and ?. operators a lot more, even inside class implementations?

[fkleuver]

This may be common in modern web-dev but it will take me some time to get used to the idea that I can't control the initialization of my class via the constructor.

You absolutely can control the initialization via the constructor. I'm not sure what makes you believe otherwise.

We have strict mode enabled in our repository which forbids any uninitialized properties, and we have hundreds of classes that have a mixture of DI-injected properties and manually-initialized properties. Zero problems there whatsoever. All fully initialized in the constructor. Example:

export class Auth {
  public user: string;

  constructor(private readonly api: SomeApi) {
    this.user = undefined;
  }
}

Same thing as:

export class Auth {
  public user: string = undefined;

  constructor(private readonly api: SomeApi) {}
}

You can also override the constructor at registration level, for example:

.register(Registration.singleton(Auth, (() => new Auth(undefined)) as any);

Or, if you need to get fancier:

.register(Registration.callback(Auth, (handler, requestor, resolver) => {
  if (!(resolver as any).auth) {
    const api = handler.get(SomeApi);
    (resolver as any).auth = new Auth(undefined /* or some other default value for user */, api);
  }
  return (resolver as any).auth; // yeah we still need to fix the types here
});

Lastly, if you just want to manually construct a global instance of something in the simplest possible way, not using DI to instantiate it but still making it available via DI, you can do this:

.register(Registration.instance(Auth, new Auth(undefined));

[xenoterracide]

Duplicate of #840 ?

[fkleuver]

There is some overlap, but the focus here is on the behavior of DI with parameter initializers

[fkleuver]

This is fixed

[Sayan751]

Closing this, as it seems to be fixed.