Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@tracked: lazy evaluation #18118

Open
buschtoens opened this issue Jun 19, 2019 · 4 comments
Open

@tracked: lazy evaluation #18118

buschtoens opened this issue Jun 19, 2019 · 4 comments

Comments

@buschtoens
Copy link
Contributor

While investigating #18075, I noticed a minor issue, that persists even after the change to WeakMap in #18091, because of the lazy evaluation

class Foo {
  @service someService;

  @tracked someProp = this.someService.serviceProp;
}

Assume Foo gets created by a container lookup or get manually called
setOwner on.

Intuitively I would expect the above code to work. And it does.
If this.someProp is only accessed later in the template or in init, everything works fine.

However, if you already access this.someProp in the constructor, you get a TypeError, because this.someService is undefined.

While I understand why, I still think this is inconsistent. If we decide to keep lazy evaluation of the initializers, we need to document it well. Would the new static decorators proposal support lazy evaluation in the first place?

@pzuraq
Copy link
Contributor

pzuraq commented Jun 19, 2019

@buschtoens this should work in all cases for framework classes, all base classes should be assigning the owner in the root constructor, before any subclasses assign their fields. Are you manually assigning the owner to a class?

@buschtoens
Copy link
Contributor Author

Yes, in this case the class was not created through the container, but instantiated manually. Should we declare manual instantiation an anti-pattern and instead require the creation through the container?

@pzuraq
Copy link
Contributor

pzuraq commented Jun 26, 2019

If you need access to the container, I would tend to say yes. The way most container classes work now is essentially:

class GlimmerComponent {
  constructor(owner) {
    setOwner(this, owner);
  }
}

This is how all subclasses are able to immediately access the owner and injections. You could setup your own classes to do something like this, or you could avoid accessing injections until after you assign the owner in general. Unfortunately, we cannot safely give the owner to a class in any other way, so you do have to do all access after setOwner is called somewhere 😕

@buschtoens
Copy link
Contributor Author

Cool. I can live with that. 👍

Maybe we should add a note about this somewhere in the guides? I can imagine that more people will start instantiating native utility classes themselves, now that it's officially supported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants