Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Defining scope type explicitly in a readable way #13677

Closed
iamisti opened this issue Jan 5, 2016 · 4 comments
Closed

Defining scope type explicitly in a readable way #13677

iamisti opened this issue Jan 5, 2016 · 4 comments

Comments

@iamisti
Copy link

iamisti commented Jan 5, 2016

Hi Guys,

Sorry if the following question will be stupid but I need to know something.
When we defining scope types in our directives it can be 3 different types:

  • scope: true
  • scope: false
  • scope: {}
    I'm always struggling when I need to create or just read a directive's cope definition if it's shared or prototypical inherited. Also it's not a good pattern in general neither that you have to simply pass boolean variables either to a function or a customisable parameter that can have different states.

My preferred approach would be something like this:

  • scope: Scope.INHERITED
  • scope: Scope.SHARED
  • scope: {}

In their implementation they can have the value of true and false (e.g.: it won't be a breaking change), it's just a matter of defining it and reading it.

I know we don't have a global variable like Scope, but I'm open for any suggestion or solution that can give me more readable format. Maybe it could be angular.Scope.INHERITED or angular.ScopeTypes.INHERITED?

Please tell me if it does make sense for you and not just for me. It'd help me a lot for readability and for stop keep searching for the true/false meanings when I need to create a new directive.

@gkalpak
Copy link
Member

gkalpak commented Jan 5, 2016

It's really a matter of interpretation. It's something you need to learn how to interprete (as many things in the DDO) and I don't think Scope.INHERITED/SHARED would be different in that respect either.

If that helps, I think they way the scope property was meant to be interpreted is:

Does this directive needs a scope of it's own ?

  • false: No, it doesn't => No scope is created. It still has access to all scopes in the view, because that is what happens anyway in Angular (independent of directives).
  • true: Yes, it needs a scope => A new (normal) scope is created. In angular, a scope (normally) prototypally inherits from it's parent scope. (Again, nothing directive-specific here.)
  • {...}: Yes, it needs a (special, isolate) scope with specific properties bound on it. (The rest is history...)

Given that we can't replace the current implementation (as it would be too big of a breaking change), one could consider adding your suggestion as an alternative, but that would only add to the complexity of the DDO and the related concepts and imo do more harm than good.

That said, if it makes sense for you, you can easily create and use your own variables (either global or injected) and use them instead. E.g.:

// Top-level global
var ScopeType = {INHERITED: false, SHARED: true};
// or
// Global under `angular`
angular.ScopeType = {INHERITED: false, SHARED: true};

app.directive('fancyStuff', function fancyStuffDirective() {
  // DDO
  return {
    scope: ScopeType.SHARED,   // or: angular.ScopeType.SHARED
    ...
  };
});

// or even as DI'ed constant
angular.
  module(...).
  constant('ScopeType', {INHERITED: false, SHARED: true}).
  directive('fancyStuff', function fancyStuffDirective(ScopeType) {
    // DDO
    return {
      scope: ScopeType.SHARED,
      ...
    };
  });

I'll leave this open for a while, to see if anyone if feeling too strongly about adding something like this :)

@iamisti
Copy link
Author

iamisti commented Jan 5, 2016

@gkalpak I understand your explanation, but still there is a mind-mapping work you have to do each time, since it's not explicitly saying which scope type do you use in that particular directive.

I'm curious what do the guys feel about it. I can also do it for my own project, but the main reason was to improve if it make sense to help others as well.

Thanks for your detailed and well-explained answer tho!

@Narretz
Copy link
Contributor

Narretz commented Jan 5, 2016

This was shot down once, so I don't think we're gonna do it: #9125

@gkalpak
Copy link
Member

gkalpak commented Jul 6, 2016

Considering that:

  1. We can't replace the current implementation, so implementing this on-top will create more fragmentation and confusion,
  2. It is super-easy to implement this on your own (or even as a reusable module) and
  3. Nobody seems to be feeling too strongly about it 😉

I'll go ahead and close it as "won't fix" 😄

@gkalpak gkalpak closed this as completed Jul 6, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants