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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ability to define root scope per Aurelia instance #664

Open
bigopon opened this issue Oct 6, 2019 · 7 comments
Open

Ability to define root scope per Aurelia instance #664

bigopon opened this issue Oct 6, 2019 · 7 comments

Comments

@bigopon
Copy link
Member

@bigopon bigopon commented Oct 6, 2019

馃挰 RFC

Ability to have a global root scope to enrich view expressions.

馃敠 Context

Sometimes I want to do this in my view: Math.random()/ Array.from(someVariables) but it can't be done since scopes does not have access to global.

馃捇 Examples

A html only custom checkbox element, that wants to generates an ID for each internal label/checkbox pair, being able to have access to Math.random() will make this simpler

Maybe do it in start task and inject IRootScope?

@fkleuver

This comment has been minimized.

Copy link
Member

@fkleuver fkleuver commented Oct 6, 2019

Just to add to this from our discussion on discord, it could be nice to also allow NewExpression syntax. In conjunction with exposed class constructors that would allow you to new up an instance of any registered constructor.

I'm not quite convinced of doing this via start tasks or in any other way limit this to configuring at root level. The way I'd see this is that we introduce a new resource type for it, just like ValueConverter and BindingBehavior, perhaps ExposedType or something like that. And it could be registered at container or at any component via dependencies just like other resources.

Then we would need to add something to BindingContext that it checks for any such registered types after it failed to find a variable.

Thoughts?

@EisenbergEffect

@EisenbergEffect

This comment has been minimized.

Copy link
Member

@EisenbergEffect EisenbergEffect commented Oct 6, 2019

I do get nervous when we enable so much in the view that people start putting tons of behavior in there, when it really should be in the view-model. However, In au1 we had a hook that you could use to add things to the binding context. I used it for enums actually sometimes. So, I can see the use case for sure. Just want to be careful about how we provide guidance around using this and also what expectations people may have about observation in these scenarios.

@Vheissu

This comment has been minimized.

Copy link
Member

@Vheissu Vheissu commented Oct 6, 2019

Furthermore, adding to this RFC that I support. In v1 to expose things like Math and other built in globals, I would end up aliasing them on the view-model anyway to use in my views. And more recently, using view engine hooks to automatically do it. A way to do this in a more official way would be awesome, but if not, I'll probably continue using view engine hooks.

@fkleuver fkleuver added this to Triage in Work queue Oct 7, 2019
@fkleuver fkleuver moved this from Triage to Backlog in Work queue Oct 7, 2019
@fkleuver

This comment has been minimized.

Copy link
Member

@fkleuver fkleuver commented Oct 7, 2019

Just want to be careful about how we provide guidance around using this and also what expectations people may have about observation in these scenarios.

This is crucial indeed.
My initial idea is an API along the lines of this:

export function expose(
  // The value to expose (e.g. a window global or a constructor fn)
  value: any,
  // The key with which to make it available in binding context
  key: string,
  // Whether or not member expressions on the value should be observed
  observeMembers: boolean = false,
) { /* ... */ }

Examples:

global

// Just want the Math api available, but no observation on its properties
au.register(Registration.expose(Math, 'Math')

local

@customElement({
  dependencies: [
    // Just want a constructor available so you can `new` it up, but no observation on its properties
    Registration.expose(MyComponent, 'MyComponent'),
    // Expose some module-scoped State object and observe its properties
    // (pretty much equivalent to this.State = State during binding, except that it becomes
    // available in all child components as well)
    Registration.expose(State, 'State', true),
  ]
})
export class AnotherComponent {}
@jwx

This comment has been minimized.

Copy link
Member

@jwx jwx commented Oct 8, 2019

Could we make it so that view expressions also try to resolve properties not found in the view model by looking in the view model's container? That way you'd just add the class constructor as-is to dependencies, global or local. Or are there good reasons not to do this?

@fkleuver

This comment has been minimized.

Copy link
Member

@fkleuver fkleuver commented Oct 9, 2019

I've thought about this some more.
I can see it might make sense to say "if it can't be found in scope, try the container's keys" in some situations.

The main problem I see with it is the effect on expressions like thing.bind="foo || bar":

  1. You're iterating the container's keys for every variable you're evaluating which is undefined. This can have a huge performance impact
  2. You might get a value from the container when you aren't expecting it, if your (possibly undefined) variable happens to share name with a registration (which could be a core framework registration or that of any plugin author)

This makes it not a good default, so at least you'd need the ability to specifically turn it on (per component).

But that brings us back to those 2 points. Usually you only need 1 or at most a handful of registrations for specifically the router. You don't need the other dozens or even hundreds that typically sit in the container. So you get unnecessary exposure to variables you don't need and unnecessary performance overhead that comes with it.

With all that in mind, I do genuinely think this is the best solution:

export class MyComponent {
  private readonly OtherComponent = OtherComponent;
}

It's a one-liner, it can be "traded" for the dependencies (which isn't needed when you expose it in the binding context), it's fast and side-effect-free.

So, I don't really see much added value in adding container to the "scope tree" and I do see a lot of potential issues.

@3cp

This comment has been minimized.

Copy link
Member

@3cp 3cp commented Oct 9, 2019

au.register(Registration.expose(Math, 'Math')

I like this. It makes easy to build a plugin that expose some global JS features. au.register(ExposeSomeJsGlobals). We can even ship an official plugin to expose selected globals which can satisfy 99% of the users.

But, I like this more:

export class MyComponent {
  private readonly OtherComponent = OtherComponent;
}

Which is exactly what I am doing in vcurrent to expose some global, without asking any feature from Aurelia framework itself... You can save some code by using this with inheritBindingContext together...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Work queue
  
Backlog
6 participants
You can鈥檛 perform that action at this time.