Skip to content
New issue

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

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

Already on GitHub? # to your account

Proposal: The internal modifier for classes #5228

Open
rbuckton opened this issue Oct 12, 2015 · 94 comments
Open

Proposal: The internal modifier for classes #5228

rbuckton opened this issue Oct 12, 2015 · 94 comments
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@rbuckton
Copy link
Member

The internal modifier

Often there is a need to share information on types within a program or package that should not be
accessed from outside of the program or package. While the public accessibility modifier allows
types to share information, is insufficient for this case as consumers of the package have access
to the information. While the private accessibility modifier prevents consumers of the package
from accessing information from the type, it is insufficient for this case as types within the
package also cannot access the information. To satisfy this case, we propose the addition of the
internal modifier to class members.

Goals

This proposal aims to describe the static semantics of the internal modifier as it applies to members of a class (methods, accessors, and properties).

Non-goals

This proposal does not cover any other use of the internal modifier on other declarations.

Static Semantics

Visibility

Within a non-declaration file, a class member marked internal is treated as if it had public
visibility for any property access:

source.ts:

class C {
    internal x: number;
}

let c = new C();
c.x = 1; // ok

When consuming a class from a declaration file, a class member marked internal is treated as
if it had private visibility for any property access:

declaration.d.ts

class C {
    internal x: number;
}

source.ts

/// <reference path="declaration.d.ts" />
let c = new C();
c.x = 1; // error

Assignability

When checking assignability of types within a non-declaration file, a class member marked
internal is treated as if it had public visibility:

source.ts:

class C {
    internal x: number;
}

interface X {
    x: number;
}

let x: X = new C(); // ok

If one of the types is imported or referenced from a declaration file, but the other is defined
inside of a non-declaration file, a class member marked internal is treated as if it had
private visibility:

declaration.d.ts

class C {
    internal x: number;
}

source.ts

/// <reference path="declaration.d.ts" />
interface X {
}

let x: X = new C(); // error

It is important to allow assignability between super- and subclasses from a declaration file
with overridden members marked internal. When both types are imported or referenced from a
declaration file, a class member marked internal is treated as if it had protected visibility:

declaration.d.ts

class C {
    internal x(): void;
}

class D extends C {
    internal x(): void;
}

source.ts

/// <reference path="declaration.d.ts" />
let c: C = new D(); // ok

However, this does not carry over to subclasses that are defined in a non-declaration file:

declaration.d.ts

class C {
    internal x(): void;
}

source.ts

/// <reference path="declaration.d.ts" />
class D extends C {
    internal x(): void; // error
}
@rbuckton rbuckton added the Suggestion An idea for TypeScript label Oct 12, 2015
@RyanCavanaugh RyanCavanaugh added the In Discussion Not yet reached consensus label Oct 12, 2015
@kitsonk
Copy link
Contributor

kitsonk commented Oct 14, 2015

Would the following also error:

declaration.d.ts

class C {
    internal x(): void {};
}

source.ts

/// <reference path="declaration.d.ts" />
class D extends C {
    x(): void {}; // error?
}

How does this differ from the more common language semantic protected?

The non-goal is not to address this in other declarations, but couldn't this start to becoming confusing for someone trying to utilise a package or library? For example, I would assume the following you would want to work this way:

declaration.d.ts

interface InterfaceC {
}

class C implements InterfaceC {
    internal x(): void {};
}

source.ts

/// <reference path="declaration.d.ts" />
class D implements InterfaceC {
}

const c: C = new D(); // error, but why? I implemented the interface properly.

When targeting ES6, do you see any changes, considerations in the emit?

Do you have a more practical use case of where this pattern would resolve significant problems? I can understand how for a consumer it could clean up an API, but at the same time, it could be really confusing where you are "hiding" things that will always exist at runtime from compile time. It almost seems like a six of one, half a dozen of another.

@jbondc
Copy link
Contributor

jbondc commented Oct 18, 2015

An alternative to modifiers is:

package.d.ts

/// <reference path="declaration.d.ts" />
internal {
   C.x;
}

An internal {} block marks which parts of the public api are hidden.

@sccolbert
Copy link

+1 to something like this

@alisabzevari
Copy link
Contributor

+1 for this.

@ToddThomson
Copy link
Contributor

Internal modifier for classes and their properties/methods within a component/program would allow greater scope for Typescript identifier shortening.
+1 for this proposal.

@shmuelie
Copy link

I think I'd want to extend internal to also work on classes.

It would work pretty much like the --stripInternal compiler flag works now, except without the need to add JSDoc to the class.

Additionally, though this may need some exploring, it could be used to keep classes within a module. So for example:

//file1.ts
module A.B
{
    internal class C { }

    var c: C = new C(); //Works
}

//file2.ts
module A
{
     var d: A.B.C = new A.B.C(); //Error because C is only available inside A.B module scope.
}

//file3.ts
module A.B
{
    var e: A.B.C = new A.B.C(); //Works, back in scope
}

@RyanCavanaugh RyanCavanaugh changed the title Proposal: The internal modifier Proposal: The internal modifier for classes Jan 28, 2016
@tetsuharuohzeki
Copy link
Contributor

I'm polishing classes' members' openness of RxJS like ReactiveX/rxjs#1244, then I wanted this feature to control openness in a complexed class dependencies.

@JonathanMEdwards
Copy link

+1 Having to write @internal all over the docs is currently my biggest pain point with TS. Which is another way of saying that things are pretty good - good job guys :)

@TheLevenCreations
Copy link

+1
any progress on this?

@bartzy
Copy link

bartzy commented May 16, 2016

+1, this would be awesome to have.

@jelbourn
Copy link

This would be really nice for Angular components where you need properties/methods to be public to be accessed by the template, but want them to be private to other users that are consuming your components. The current /** @internal */ feature is problematic because it causes types to fail to satisfy implemented interfaces when the d.ts is compiled downstream.

@kito99
Copy link

kito99 commented Jul 1, 2016

+1 This would really help with testing; right now you have to make methods public that you really just want to be visible for testing but not part of the public API.

@davidstellini
Copy link

Agreed. This would make sense for services, where you can have internal methods that can only be accessed from other services if necessary, but not the API consumer.

@jmcota
Copy link

jmcota commented Jul 12, 2016

+1 internal modifier would be a big Christmas gift.

@domchen
Copy link
Contributor

domchen commented Aug 3, 2016

We really need this feature too. I am a developer of a HTML5 engine called Egret. Our engine contains plenty of internal APIs which are not allowed to accessed by user, otherwise it may cause internal problems. Currently, we have to make them as public and add some documents for it says ' do not use this method! '. It is really hard for us to design a safe and stable engine API because of the lacking of internal . Hope that you could add this feature soon, thanks in advance :)

@zpdDG4gta8XKpMCd
Copy link

excuse me, i know what a module or a namespace is, but what is a package?

@zpdDG4gta8XKpMCd
Copy link

related #321

@andykais
Copy link

andykais commented Mar 27, 2020

I have another thought here. From what I understand, an internal modifier means that method is accessible until it is used by an outsider. But, what is an outsider? I assume we just mean after typescript has built a project. So, what if we had a simple @stripmethod doc comment above methods? Then when typescript declarations are built we remove those methods from the declarations. I think both defining the internal methods and marking them as 'ignored' in declaration files feels like unnecessary complexity. If we simply strip the methods, we might even be able to handle this from outside typescripts compiler.

[edit]
Wait. Is this solved already? There appears to be a --stripInternal compiler option https://stackoverflow.com/questions/32188679/how-does-a-typescript-api-hide-internal-members.

//Class will not be visible
/** @internal */
export class MyHelperClass {

}

export class MyPublicClass {
    //Method will not be visible
    /** @internal */ 
    helperMethod() {}
}

// Binding will not be visible
/** @internal */
export const MyValue = 5;
tsc --stripInternal

@jgranick
Copy link

@andykais If the project is distributed as d.ts then yes, this hides the properties but the properties are still visible in IntelliSense when working with .ts. If you use a common prefix such as "_" then it is sorted before your public members.

I propose that if an internal keyword were adopted, it would behave like /** @internal */ when generating type declarations, public when compiling but protected in IntelliSense hints.

@EisenbergEffect
Copy link

@jgranick By protected do you mean that it will not show up in intellisense?

My preference for internal would be:

  • Works as public within the project where the code is defined.
  • Is stripped from all d.ts files.
  • Is completely invisible outside the TS project where it is defined, regardless of whether it's a TS project that consumes the library directly or something that consumes the d.ts.

Another way I think of it is that it's a normal public property/method/etc in JavaScript. There's no runtime component to this. It's just that it's completely hidden by compiler/intellisense/tooling for any code outside of the project where it is defined.

@jgranick
Copy link

@EisenbergEffect I would be happy with your proposal if the concept of "inside" or "outside" the project is clear to the compiler. I was concerned this was a foreign concept?

If there is not a way to distinguish cleanly between "inside" or "outside" the project then my proposal might be a second best:

  • Works as public.
  • Is stripped from all d.ts files.
  • Is completely invisible to IntelliSense (or has protected visibility)

@johncrim
Copy link

johncrim commented Apr 13, 2020

I think it's been mentioned elsewhere in this thread, but I would add to @EisenbergEffect's description:

  • internal class, function, and variable names should be replaceable when aggressively minifying, eg using Closure's ADVANCED_OPTIMIZATIONS .

This is both a way to provide a level of runtime enforcement for internal types/functions/variables, and provides a runtime benefit (smaller names) with no downside. It's also part of the reason that I think internal should be standardized (so tools can use the additional information that it provides).

@falcon03
Copy link

falcon03 commented May 5, 2020

Just my +1 for internal. I was surprised to find out that Typescript does not support this :(

Another use case to consider, which is specifically mine ;), is having classes share a common abstract parent class. You may want to distribute/document their children, but neither the parent itself nor its methods/properties...

@trusktr
Copy link
Contributor

trusktr commented Jun 6, 2020

You may want to distribute/document their children, but neither the parent itself nor its methods/properties...

That's simlar to "package protected" as called in other languages. What we need in JS and TS is "module protected" and "module private". Seems that #35554 would suit your needs more, and it is better than the internal idea here because it limits scope of variables to certain modules, rather than making everything visible in the whole project (protected and private is still valuable within projects, and replacing protected with internal makes them public within a project which is not ideal).

@jimsugg
Copy link

jimsugg commented Jul 17, 2020

+1 for internal.
I would also like to see this for global functions, variables, etc which also have the export keyword, so that they can be accessed from other files in the same package, including tests (which requires an export declaration,) but 'hidden' externally. The internal keyword would communicate clearly to future maintainers the intent of the associated construct.

On our team, we use the jsDoc /** @internal */ comment quite a lot to designate internal functions, but there are some files that we exclude from the compilation used to generate the index.d.ts file, and in those files, we don't always consistently use the @internal comment (though we probably should.) If someone in the future then does something such that those files are included in the index.d.ts compilation, those functions would be exposed.

An internal keyword with the characteristics described would be especially useful to better communicate intent, and to enforce it as best as Typescript can.

@Xample
Copy link

Xample commented Dec 19, 2020

The only time I need to access private methods from the outside would be within a unit test. As we separate the tests from the code how would the "internal" keyword cope with this ?

@lorenzodallavecchia
Copy link

@Xample Real private methods should not be tested directly, since they are implementation details.
However, it may happen that some methods are "external" from the perspective of a given class, but still "internal" with regard to your libraries. In these cases, it does make sense to test them and this is the exact scenario where the internal keyword would be useful.

So basically you would use internal for all things that are part of the API of a single .ts file, but not part of the API of the overall library that you are building.

@KostyaTretyak
Copy link

Real private methods should not be tested directly, since they are implementation details

Please, see this comment.

@Lusito
Copy link

Lusito commented Dec 29, 2020

Since this seems to go nowhere anytime soon and I am currently in need for this, as I am maintaining a TypeScript Port of a C++ Project (Box2D), which uses C++ friend classes, I need an alternative way to strip away access from the outside.

So I've written a small post-processing tool to help me do this by using recast to transform the (generated) .d.ts files after they've been built. This works by adding a jsdoc tag @internal to your properties, methods and exports.

This is better than stripInternals, since the identifiers still remain in the d.ts files, so they will complain on extended classes, etc. Of course, you'll have the extra work of using jsdoc, but an internal access level would require almost the same amount of work and when the language feature gets implemented, you'll have it easier by just searching for the jsdoc tag and replacing it. Could probably even write a migration tool for that. Doesn't seem to be that much work with recast.

I still need to add these jsdoc tags to my Box2D port, so I haven't tried this on a large scale project yet, but the sample snippets work just fine. Give it a try if you like: https://github.com/Lusito/idtsc

Feel free to add feedback in the project's issue tracker. Check out the readme for a sample input/output

Looking forward to a language feature, so I can discontinue this project.

@woldie
Copy link

woldie commented Apr 10, 2021

I like this concept of an internal access specifier for the reasons @johncrim indicates because a symbol marked as an "internal detail" can have its face be ripped off and be minified/uglified to death. That is a pretty nice optimization hint!

I have a separate concern that I feel may have been conflated a bit in the overall discussion above and would like to disambiguate it here.

While TDD'ing code, I will place my *.test.ts unit test files in the same directory as their systems under test, which seems to be a fairly common convention.

I'd like to be able to mark some methods of my class with an access specifier other than public and still have those members be visible within the test module. JSDoc has this concept, known as package-private: https://jsdoc.app/tags-package.html

The idea is that modules in the same directory can access the class member as if it was public, but it is private to modules in every other directory. I like the terminology JSDoc uses for this access specifier, package, so I propose that be what Typescript uses as its keyword unless you have other intents for package.

Finally, this concept differs from #321 where the proposal is for a module access specifier, which is to be module-private. I imagine that internal could be mixed with access specifiers public, private, protected, package and module. And if another access specifier is not mixed with internal, public should be the implicit access level as @EisenbergEffect recommends.

@cmcnicholas
Copy link

still greatly required, we have a multi project solution with TypeScript (using tsconfig project references) and currently we leak all the "should be internal" methods into the other projects, we cannot mark them as private as they are required to be used internally by the project that owns them. internal operating similar to c# would greatly improve the review/maintainability of this codebase and I don't believe I am the only person in this situation otherwise why would TypeScript project references be advertised as a good solution to separate concerns?

@ReeceGordon
Copy link

+1 for this.

@LukeBrandon
Copy link

LukeBrandon commented Jun 3, 2021

+1 for this as well.

@tylerthecoder
Copy link

I think this would be very helpful +1

@jr-grenoble
Copy link

While working on a set of cooperating classes, I was looking for the equivalent of C++ friend classes, and stumbled on this internal proposal. This mechanism is actually much better than friend as it encourages encapsulation at the module level as well as splitting code into small modules of related functionality.

@FilipeBeck
Copy link

If we simply strip the methods, we might even be able to handle this from outside typescripts compiler.
Wait. Is this solved already? There appears to be a --stripInternal compiler option

The internal modifier would be a syntax sugar for /* @internal */. I think this is the best solution.

While working on a set of cooperating classes, I was looking for the equivalent of C++ friend classes...

I like this possibility too.

@lorenzodallavecchia
Copy link

The internal modifier would be a syntax sugar for /* @internal */. I think this is the best solution.

I'd like to add that a huge shortcoming of using /* @internal */ today is that it is "dumb". It just erases members from the final .d.ts files, with no regard for API correctness.
The stripped .d.ts files may contain errors like:

  • methods referring to missing types in their signatures,
  • import..from declarations that refer to empty .d.ts files, which a downstream project will not recognize as valid ESM.

The only way to protect from those issues today is to use a tool like api-extractor for checking that the API is consistent.

Ideally, the new internal keyword should do the consistency checks while checking the source files and emit the necessary diagnostics. For example, on encountering a non-internal method using an internal type, TypeScript would advise the programmer to either

  • making the method internal too,
  • remove internal from the offending type,
  • use on the method signature a different (wider) type that is not internal.

On a related note, a pain point would be having to rewrite all those internal type aliases that we declare just to make our code more readable, but that we don't want to export from our files. Maybe TypeScript should automatically expand those internal type aliases into their definition.

The interaction between internal and API consistency is the reason I think this issue has something in common with definitions bundling (#4433).

@thesoftwarephilosopher
Copy link

thesoftwarephilosopher commented Nov 4, 2021

One idea that would be even more explicit and not require any special file/directory handling, would be if I could mark a specific method/property as being allowed by specific classes, e.g.:

class Bar {
  run(foo: Foo) {
    foo.run() // good
    foo.run2() // type error
  }
}

class Baz {
  run(foo: Foo) {
    foo.run() // type error
    foo.run2() // type error
  }
}

class Qux {
  run(foo: Foo) {
    foo.run() // good
    foo.run2() // good
  }
}

class Foo {
  // Foo and Qux could be in the same file or imported from anywhere
  internal(Bar, Qux) run() {
    programming.solveOneOffError(n => n+1);
  }

  // different methods/properties of the same class could be internal to different things; they'd all be independent
  internal(Qux) run2() {
    this.jog();
  }
}

@TurboEncabulator9000
Copy link

TurboEncabulator9000 commented Nov 2, 2023

I like @sdegutis' idea, which would essentially be internal that can be narrowed to act more like friend as needed. You'd get the best of both worlds.

Some seem to think this proposal should be exclusive of the friend idea because it assumes only one useful boundary (package level) for visibility control. I say there are two such boundaries. Inappropriate coupling can come from inside a package at least as easily as it can come from outside.

As originally proposed, internal would help protect members from inappropriate access from outside a package, but would do nothing to protect them from inappropriate access from within the same package. If anything, people working on a package will naturally encounter more opportunities to misuse access than someone whose only exposure to the package is that they typed npm i and read about the public interface on GitHub.

Now, if your project is 100% yours, or is only worked on by you and other people who understand a unified vision, maybe it makes no difference to you. If that's you, you're very lucky, and I envy you.

In reality, a big organization is going to have a lot of developers who have varying levels of understanding of good architecture, design patterns, etc. Some sleep with the Gang of Four book under their pillows. Others heard of design patterns once three years ago, and... that's it. Therefore, I allege that it's useful to have a mechanism for sharing access to certain things within a subset of a package, but not necessarily to the whole entire package.

For example, suppose you have three layers:

  1. Controller
  2. Service (instantiated by Controller)
  3. Adapter (factoried by Service)

Service has some methods useful both to itself and to Adapter, but which would be inappropriate to be called by Controller. Lacking friend or some substantially similar method, those methods could be made public so that Adapter can call them.

Of course, sooner or later, someone comes along who doesn't understand this design, isn't inclined to learn, and is used to adding conditionals with additional code in the most convenient-looking places. Their behavior is dictated by an energy function which only cares about how many points they can burn down in this sprint. More points = more better! Whatever happens in three months or a year is irrelevant to this energy function.

They tack on some code to Controller which calls methods on Service which were intended for use only by Adapter. This makes perfect sense to them. You know it's going to be harder to maintain this way, but they don't, and whoever reads their PRs won't care either. You can't read every PR organization-wide, and you may not be empowered to decline them anyway. Now a Controller that should be quite thin, and shouldn't be tightly coupled with implementation details at lower levels of abstraction, is hard-wired directly into that stuff.

Possible solutions:

Service "eats" Adapter. Adapter's properties and methods are all moved into Service. What would otherwise have been a concretion of Adapter is now a concretion of Service. This would mash two design patterns together in a way that isn't so great. The resulting class would be much less compliant with the Single Responsibility Principle. What might be just over the edge of "uncomfortably large" today may sprawl into a monstrous God Object by the time a few years have gone by.

The methods/variables you wanted to "friend" could be refactored out to some other class. Service and Adapter would have to extend, implement, or compose with that class. You wouldn't need friend or internal. If you use the extend/implement approach, at least Controller can't mess with those things directly, but then you have to play with inheritance in a way you wouldn't have to with friend or internal. If you use composition instead, then Controller is free to import the class you only wanted to be used by Service and Adapter.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests