Skip to content
This repository has been archived by the owner on Feb 22, 2018. It is now read-only.

Disallow private fields without leading _ #278

Open
yjbanov opened this issue Sep 3, 2015 · 3 comments
Open

Disallow private fields without leading _ #278

yjbanov opened this issue Sep 3, 2015 · 3 comments

Comments

@yjbanov
Copy link
Contributor

yjbanov commented Sep 3, 2015

Time to fix this old TODO: https://github.com/angular/ts2dart/blob/6f3bbd896e7e712049eb4f2403e67f59932cf697/lib/declaration.ts#L426

@vicb
Copy link
Contributor

vicb commented Sep 3, 2015

What about this pattern:

// Base and Child in the same file (Dart has library level private members)
class Base {
  _internalProp;
}

class Child extends Base {
  foo() {
    this._internalProp;
  }
}

We have some of those in the form component.

What would be the best solution here ?

  1. keep the current behavior (do not enforce private for _names),
  2. allow protected,
  3. rename _internalProp to internalProp

None of those solutions are great:

  1. _var would not be accessible outside of the current library in Dart (which === current file in Ng2)
  2. protected members would be accessible from child classes in other files in TS. Only in the current file for Dart
  3. Everything is public so we can access everything from everywhere. Bad for encapsulation and for documentation (ie what is the class API) ?

@harryterkelsen
Copy link
Contributor

@vicb
I think option 2 makes the most sense. Since _internalProp has a leading underscore then you are already susceptible to the problem of option 2 whether or not protected is allowed.

I think it makes sense to allow protected and enforce a leading underscore for protected fields.

@mprobst mprobst closed this as completed Sep 10, 2015
@alexeagle
Copy link
Contributor

I am adding a config option for this in ts2dart, in #284
It defaults to false, so we can release ts2dart to angular without breaking the build, then someone needs to make the angular changes to comply with this restriction before we turn on the enforcement.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

5 participants