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

Avoiding pollution of environment's namespace #3338

Closed
ddotlic opened this issue Jun 2, 2015 · 22 comments
Closed

Avoiding pollution of environment's namespace #3338

ddotlic opened this issue Jun 2, 2015 · 22 comments
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript

Comments

@ddotlic
Copy link

ddotlic commented Jun 2, 2015

This is somewhat related to #2923, but I'm deliberately opening a separate item because the discussion there has abruptly stopped and more importantly I don't think we're on the right track for the issue at hand.

Let's restate the issue differently: imagine a simple TS project which uses no modules or namespaces, because it's really annoying to export/import almost every class in the project. It's not a library other people will use, it's just something I use for my own site(s). I use the feature of the TS compiler to produce a single JS file and reference it from my HTML5 page(s).

However, I don't like the fact that my 'classes' are polluting the global namespace of the browser environment (had I used the project for node/server side development, the result would be similary unpleasant).

Every discussion around this issue leads (inevitably?) to modules or namespaces, but more I think about this more it seems to me we're not on a right track.

While modules and/or namespaces do allow for quite fine-grained level of control regarding visibility of 'classes', for the simple case I simply want everything visible, but I don't want it in a global namespace of the environment, I want to separate it so I don't produce clashes with other scripts running in the same environement/page. IOW, inside my project everything is 'public', outside too (I'd rather make everything visible than export/import all the time) but not in a global namespace of the environment.

Would it be at all reasonable to expect the TS compiler to support this scenario through a flag or option in tsconfig.json the final result being identical to regular compilation but wrapping everything in a IIFE while preserving source maps info? It's understood that this option would effectively 'deactivate' any and all notions of modules/namespaces (it would be invalid to use the two at the same time).

Or should I assume this will never be implemented and look into wrapping the final result myself through a custom build step (which should be possible but is slightly irritating to do when one uses sourcemaps as they will be invalidated by wrapping)?

@ddotlic
Copy link
Author

ddotlic commented Jun 2, 2015

To be clear, this is not a bug, it's - at best - a suggestion (cannot add label myself otherwise would have done it already) that probably needs refinement.

@ivogabe
Copy link
Contributor

ivogabe commented Jun 2, 2015

You could do that with a build system. Example:

var gulp = require('gulp');
var ts = require('gulp-typescript');
var header = require('gulp-header');
var footer = require('gulp-footer');

gulp.task('default', function() {
    return gulp.src('lib/**/*.ts')
        .pipe(ts())
        .pipe(header('(function(){'))
        .pipe(footer('})();'))
        .pipe(gulp.dest('release'));
});

That will place the code between (function(){ and })();, it creates an IIFE, so the global namespace isn't changed.

@ddotlic
Copy link
Author

ddotlic commented Jun 2, 2015

@ivogabe ...which is exactly what I've meant by

Or should I assume this will never be implemented and look into wrapping the final result myself through a custom build step

Btw, thanks for your excellent work on the gulp plugin for TS, I use it for all my TS projects, works great!

However, while global namespace isn't changed, I still need an "entry point" to my code, if I just wrap it in IIFE, I can't get to it any more 😄

I would need code very similar to what TS emits itself - need to create one object which represents my own "namespace", send that into IIFE and place everything in that (instead in window).

This last step in fact requires that TS compiler participates, so what I've said - and you unless I'm missing something - doesn't actually hold, I cannot (trivially) resolve this with a custom build step.

If I am missing something obvious, please let me know.

IOW, I want that TS pretends that I've written module MyMode around each and every declaration/definition in all my TS files, that I've implicitly exported everything and imported everything into each file, then emit normally. Do I make sense?

For a bit more context, please see my comment here: #2923 (comment)

@ivogabe
Copy link
Contributor

ivogabe commented Jun 2, 2015

You can wrap your code in var myLib = {}; (function(myLib){ and })(myLib); or var myLib = (function(){ and return myLib; })();. That will make myLib available in the global namespace.

@ddotlic
Copy link
Author

ddotlic commented Jun 2, 2015

@ivogabe Which is also what I've implied by

I would need code very similar to what TS emits itself - need to create one object which represents my own "namespace", send that into IIFE and place everything in that (instead in window).

I should have been more explicit.

I started writing how this will not work still, but then starting executing code mentally and now I'm not sure any more 😄 Let me try in a test project.

Also, don't forget that wrapping is 'easy', I need to preserve sourcemaps, and for that I need gulp plugins which can take sourcemaps from a previous transformation step and modify so that the next one is still valid (hopefully the plugins you've mentioned are compatible with gulp-sourcemaps plugin).

@ddotlic
Copy link
Author

ddotlic commented Jun 2, 2015

@ivogabe ...and, of course, they appear not to be compatible, based on the list here. Darn.

@ddotlic
Copy link
Author

ddotlic commented Jun 2, 2015

@ivogabe I don't see how the proposed solution would work.

For a trivial case where I have two empty classes (in TS), they both become local variables of the wrapper object function, so they aren't globally visible as members of myLib.

I definitely think we need TS compiler to participate here.

Or, I could of course add namespace myLib (or module myLib) around all my classes, then export whatever I reference (even "inside" my TS project, compiler won't recognize symbols which aren't exported even if they are in the same namespace/module) and need to "see". This will work, but is ceremeny I was hoping to avoid.

@ddotlic
Copy link
Author

ddotlic commented Jun 2, 2015

@ivogabe If you look at my original example (with dummy classes A and B), try to compile without, then with module (example code is provided); I wanted to use 'namespace' to avoid confusion with modules, but for some reason, namespace keyword is not yet recognizable by 1.5-beta.

You will see that adding module does the necessary thing as "nested" symbols end up being assigned to their "namespace", and are then accessible.

But then we come back to ceremony 😄 Adding module doesn't bother me as much as "holding compiler's hand" for every bloody symbol which is already in the same namespace - I need to export each and every one, even if I don't in fact plan on exporting them (example with classes A and B where I export A only to satisfy compiler, I have no such need).

@ivogabe
Copy link
Contributor

ivogabe commented Jun 2, 2015

Missed that, sorry. You're right about the source maps, gulp-header doesn't support source maps, but there is an open issue (gulp-community/gulp-header#7) for it.

If I'm not mistaken the namespace keyword is available in 1.5, but not in the alpha and beta. I think it'll be released soon.

Would it be a solution to add everything to a private namespace and create a public namespace that will export some classes? Example:

module myLibPrivate {
    class A {}
    class B {}
}

module myLib {
    export import B = myLibPrivate.B;
}

@ddotlic
Copy link
Author

ddotlic commented Jun 2, 2015

Would it be a solution to add everything to a private namespace and create a public namespace

This sounds excessively complex for such a simple case. The main issue here is the abuse of module for something which it wasn't meant for. I had high hopes that by adding a new keyword namespace we might end up having something useful for "simple" cases like this, but apparently it does the same thing as module for "internal modules" (which I agree is very confusing name for the concept we're trying to model here).

Based on several discussions I've seen here this appears to be a pain point for quite a few devs out there, I'm surprised Microsoft hasn't already done something about it.

Ultimately, having 'namespace' then having to 'export' things only to satisfy compiler isn't too bad and if someone from TS team confirms that there are no plans to support "my" scenario I guess I'll have to go that route, I opened this issue to find out if there's any chance of having support from TS or should I do best with what we have.

@ddotlic
Copy link
Author

ddotlic commented Jun 2, 2015

@ivogabe Ultimately, when I think about the current solution, the only thing that irritates me is that I need to export things which I don't potentially want to export. Looking at my example with A and B (when wrapping with module) I only export A to satisfy compiler but it ends up being exported as any other class would be (B isn't exported, which is correct).

Of course the best would be to have a compiler flag which would implicitly wrap everything, which is equivalent to wrapping everything in a module and exporting it all, just with much less ceremony (and yes, this would mean I cannot choose what is not exported since I'm implicitly exporting everything).

@mhegazy
Copy link
Contributor

mhegazy commented Jun 2, 2015

We have talked about this as a self-executing-module. basically emitting the code with some --m sfx and --out file.js and it would emit as a single IIFE and would make it a requirement to not import or export anything.

@mhegazy mhegazy added the Suggestion An idea for TypeScript label Jun 2, 2015
@ddotlic
Copy link
Author

ddotlic commented Jun 2, 2015

@mhegazy Yes, yes, yes! 👍 👍 👍

Exactly. Will not work for some people, but will work for vast majority of simple to medium sized projects.

I just realized the reason you require exporting a class one derives from - when you don't merge declarations into a single IIFE, you're obliged to do this because you emit separate IIFE for each TS file, so the derived class cannot see the base class unless base is exported. The problem disappears when you merge all declarations into a single IIFE.

So you tentantively plan on doing this? May I be so bold to inquire at least about estimated complexity of this, if not timing?

@ddotlic
Copy link
Author

ddotlic commented Jun 2, 2015

@mhegazy You would of course need a way to specify the "namespace" to put the names into, so just --m sfx would not be enough (I'm all for single additional switch, whichever way it is implemented). It's understood that the same thing should be achievable through tsconfig.json which is worth its weight in gold for reducing ceremony (goodbye ///<reference..., we hardly knew you).

@mhegazy
Copy link
Contributor

mhegazy commented Jun 2, 2015

why would you need a namespace? i am thinking of something that is not exported, i.e. not imported by any one, it is more of a side-effect-only-module; consider something like like the ts compiler for instance, it has something like;

module ts {
    ....
}
ts.executeCommandLine(ts.sys.args);

this would emit as:

(function() {
    var ts = ...
    ts.executeCommandLine(ts.sys.args);
})();

there is nothing visible from outside, nor does it consume any other modules from the environment..

@ddotlic
Copy link
Author

ddotlic commented Jun 2, 2015

I see what you're saying, we're not talking about the same scenario.

I do want to use stuff I've built from the "outside", and I'm not against using the newly added namespace (I prefer to avoid module to avoid confusion) keyword if need be and having to export things I want visible from the outside, but I am against having to export classes that I have no intention of exporting just to help compiler work around JS deficiencies 😉

Let's see on a concrete example, the one I've used before

// in file A.ts
namespace MyLib {
  class A {
  }
}

// in file B.ts
namespace MyLib {
  class B extends A {
  }
}

// in class C.ts
namespace MyLib {
  export class C {
  private b = new B();
}
}

So my intention is to export C but not A and B. This is strictly speaking not identical to what I've described above, I'm assuming we're past that and have to rely on the existing behavior, I'm making do with what we have today.

Building this today with --out mylib.js results in compilation error. The only reason for error as far as I can understand is that emitter cannot create correct code if each file's code is wrapped in a separate IIFE, which is the case today. Adding export to A and B works, but makes them too visible from the outside as undesired side-effect.

If you merged things into a single IIFE, this should compile and work like a charm, no? Only C would be visible and the rest nicely hidden, all while respecting JS silly visibility rules.

Alternatively, you could also export everything given a "namespace" (my original ask), using the same approach - no exports (but everything is exported implicitly), no namespaces (just a single one given on a command line or in tsconfig.json), everything in one IIFE.
This second approach is a blunt tool, but should work for a lot of people who don't care about purism and just want to get the sh*t done 😉

@mhegazy
Copy link
Contributor

mhegazy commented Jun 2, 2015

I think i see what you mean. I believe the change needed here is for the compiler to support bundeling for external modules, and then you would use ES6 module style exports by doing something like:

// in file A.ts
export class A {
}

// in file B.ts
import {A} from "A";

export class B extends A {
}

// in class C.ts
import {B} from "B";

export class C {
  private b = new B();
}

// in Public.ts
export {A} from "A";
export {C} from "C";

// B is not exported from this module

then the compiler would bundle all your modules to give you a single .js file that contains all the three files, and use Public as the entry point, thus only exporting A and C and not B. the same would apply to .d.ts generation that would not have any reference to your B.

@ddotlic
Copy link
Author

ddotlic commented Jun 2, 2015

@mhegazy If you look at my example, both A and B should not be exported. I find it ironic how you use export in your example in order to not export things 😉

I actually don't want any of the module stuff as that's not what I'm trying to say here.

I have three classes, one needs to be visible after compilation, two don't. I don't care what the mechanism is, but a combination of namespace and export should be fine.

Better yet, would you prefer if I used public before class, like in C# (instead of export)? public would imply visibility in the same way, and modularity would be handled by --out and the fact that all code ends up in a single JS file in a single IIFE which wraps everthing and makes visible only stuff marked with public. Minimum ceremony, no overlap with module, export and co. and a useful thing for an average TS/JS developer building a simple library not to be used by many (except maybe inside an "enterprise").

The module discussion has polluted many a times discussion about simple visibility of things.

Besides, your last example adds so much noise (including import on top of everything else) to every class that I'd rather stick export next to each and every class and just be done with it (no offense).

@mhegazy
Copy link
Contributor

mhegazy commented Jun 2, 2015

You are looking for an internal modifier, and make that the default of unannoteated classes/functions; i.e. they are exported from the namespace, but visible only within the compilation unit.

I see your point, but we are in the JS realm here, and compilation units are not well defined in JS as they are in C# for instance (with .dlls being non-modifiable project outputs); also function closures (namespaces) and modules (ES6 modules are now the standard) are existing common JS modularity patterns.

There is already support for what you are asking for. just define all your classes/functions in one module/namespace closure and then your done; if you want to define a class per file, and do not use modules, you would need to make them accessible. changing this behavior now would be a huge breaking change that we can not take, and adding a flag to switch the visibility rule would result in forking the language, and that is something that we can not do either.

Whether to use export or public is style question, TC39 opted for export in modules to match existing JS conventions for AMD and node; and TypeScript opted to use them as well for the same reason, and also because we thought namesapaces (internal modules) to have more likeness to external modules (AMD and node) than to C# modules. that may not be the best choice in hind site, but again changing what they mean now would be a breaking change and would introduce a new concept for TypeScript users to learn.

@mhegazy mhegazy added the Declined The issue was declined as something which matches the TypeScript vision label Jun 2, 2015
@ddotlic
Copy link
Author

ddotlic commented Jun 2, 2015

@mhegazy Yes, I am looking at the internal more-or-less (not exactly but close enough).

OK, if this is a huge undertaking, then I'll just put everything in a namespace and just export it either all, or a minimum I have to. Some things will end up accidentally exported, but not a big deal.

Out of curiosity, why wouldn't it be possible to support my scenario where I did annotate classes with export and where compiler would put everything in a single IIFE (using my namespace MyLib)? Looking at the code emitted today, there would be no problem for emitter, would it be a problem for the compiler itself to recognize the names coming from the same namespace but in a different file (and not exported)? If too convoluted answer, just let me know and I'll bugger off 😄

Seeing how I'll have to now go back and annotate almost all of my classes, I'd like to do that as soon as namespace keyword becomes available. Seems like 1.5 is stable for quite some time, any idea on the release window? Thanks.

@mhegazy
Copy link
Contributor

mhegazy commented Jun 2, 2015

Out of curiosity, why wouldn't it be possible to support my scenario where I did annotate classes with export and where compiler would put everything in a single IIFE (using my namespace MyLib)?

The only problem is that each namespace introduces a lexical scope, and captures variables defined in this scope. merging multiple of these scopes means you have to resolve name conflicts, and rename things, in addition it changes the capturing rules.

Seems like 1.5 is stable for quite some time, any idea on the release window? Thanks.

We usually leave releases to stabilize for a few weeks before they are out. We have no more features for the release, just bugs, so should not be long. stay tuned :)

@ddotlic
Copy link
Author

ddotlic commented Jun 2, 2015

OK, thanks for the info. Let me "help" by closing this issue then 😉

@ddotlic ddotlic closed this as completed Jun 2, 2015
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

3 participants