Pass owner to constructor.

[rwjblue]

Ensure that services and other injected things work properly from the constructor.

[pzuraq]

👍 generally looks good. Only thing I would say is it seems weird for owner to come second, I imagine it'll probably be first in general with framework objects in the future, but I can also understand why you'd want to do this without making breaking changes.

[rwjblue]

Only thing I would say is it seems weird for owner to come second, I imagine it'll probably be first in general with framework objects in the future,

Totally agree. My thought was to force it to be two args first, then swap them in a public API in a major version bump. Which should be easier...

[buschtoens]

I just had an idea, which might be really stupid, but I would like your opinion on it. What if, instead of passing owner as an argument to the constructor, we use some global state?

Since super has to be called synchronously, we can be certain that nobody can interfere.

export const __OWNER_REFERENCE: { owner?: ApplicationInstance } = Object.create(null);

export default class SparklesComponentManager {
  // ..

  createComponent(Klass: typeof SparklesComponent, args: ComponentManagerArgs): CreateComponentResult {
    __OWNER_REFERENCE.owner = this.owner;
    let instance = new Klass(args.named);
	__OWNER_REFERENCE.owner = undefined;
    return instance as CreateComponentResult;
  }

  // ...
}

import { setComponentManager } from '@ember/component';
import { setOwner } from '@ember/application';
import { __OWNER_REFERENCE } from 'sparkles-component/component-managers/sparkles';

class SparklesComponent<T = object> {
  constructor(public args: T) {
    if (__OWNER_REFERENCE.owner) {
      setOwner(this, __OWNER_REFERENCE.owner);
    }
  }

  // ...
}

[rwjblue]

Yep, definitely would work. I'm mostly trying to match the API of the upcoming glimmer component RFC, which currently has two args (owner and args). I'll cross link the RFC once its officially submitted (hopefully in the next day or so).

[buschtoens]

RFC #416 Glimmer Components

[buschtoens]

Is there anything holding this back? Can I help?

[pzuraq]

We decided to focus on GlimmerComponent implementation, since this would either not match the API, or be a breaking change

[jordpo]

This is swapped now I believe

[mike-north]

should be

Suggested change

	constructor(public args: T, owner: any | undefined) {
	constructor(public args: T, owner?: {}) {

to describe the following constraints:
(a) undefined need not be passed explicitly under any circumstances
(b) if an argument is passed, it cannot be null or undefined

[rwjblue]

@mike-north - Seems good, should I use object or {}? (mostly just curious to the reasoning)

[mike-north]

@rwjblue

let x: object = '';  // ❌
let x: {} = '';  // ✅

If you want to allow string, number Symbol and boolean, use {}
If you just want things that are assignable to JS Objects, use object

[rwjblue]

See #33, closing in favor of using @glimmer/component directly.