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

Question: Desired behavior of triple-slash references in commonjs modules #4694

Closed
weswigham opened this issue Sep 8, 2015 · 3 comments
Closed

Comments

@weswigham
Copy link
Member

Here's an example demonstrating what I'm about to ask about.

As of the TS 1.6 beta, we've started following package references for typings! Cool! This lets us use patterns more familiar to commonjs package consumers to distribute types, which is awesome! There's in interesting issue, though. I'm ignoring the global ambient module scoping problem - let's say I plan to use proper external modules all the time.

When this is the case, if I do want to pollute the global scope, it is impossible for me to do so within the typings file found by TS. (Since it's an external module, all its types are internal except those exported.) However, by /// ref'ing a file which globally pollutes, I can presently globally pollute. Given that external modules aren't supposed to leak types, this is... awkward. This is a result of how loosely we add ///ref'd files versus those found via module resolution. While it works as a global scope pollution workaround at present, I think this behavior is something we should change/scope/specify before the full release of TS 1.6, or it could result in some bad behavior in these cases.

Interestingly, these globals don't get injected into the global scope until you add

import "typed-node";

which looks up the module (and injects the ref in the found module). Honestly, in an external module .d.ts, I'm not sure the behavior of this statement was ever really specced, which is why I'm bringing it up. The compiler would never emit this statement itself into a .d.ts from what I can tell, yet we do interesting things when we read it in one.

I have no issue with this statement in a .d.ts. Actually, I think this is quite nice from the perspective of how it gets used, using the import-for-side-effects to just pull in globals (AKA typesystem side effects) actually feels right. Here's the problem, though - when the module I've written that import in gets imported (in any way), now the containing package has that ref file and its types in its scope, too.

This means that import * as http from "typed-http" inadvertently adds Buffer to the global scope! Buffer's not even defined in that module! It's in a different package entirely! And typed-http never explicitly exported it, despite being a proper external module!

What I'd like to see is that "global" Buffer available in packages which have explicitly done import "typed-node". In effect, importing a package for side effects could merge its globals into yours (but not the other way around), and those globals are never leaked into a containing package otherwise. If this were the case, then different packages could rely on different versions of "typed-node" without fear. (Without polluting the environment of those who only access their explicit exports!)

@DanielRosenwasser DanielRosenwasser added Question An issue which isn't directly actionable in code and removed Question An issue which isn't directly actionable in code labels Sep 8, 2015
@jbrantly
Copy link

jbrantly commented Sep 9, 2015

What I'd like to see is that "global" Buffer available in packages which have explicitly done import "typed-node". In effect, importing a package for side effects could merge its globals into yours (but not the other way around), and those globals are never leaked into a containing package otherwise. If this were the case, then different packages could rely on different versions of "typed-node" without fear. (Without polluting the environment of those who only access their explicit exports!)

This seems very closely related to #4668

@jbrantly
Copy link

jbrantly commented Sep 9, 2015

@weswigham You scoped this to /// <reference />, but I think the same problem exists with import. For example:

// app.ts

import "somelib";

// node_modules/somelib/index.d.ts

import "somelibdep";

// node_modules/somelib/node_modules/somelibdep/index.d.ts

declare var SomeGlobal: string;

In the above example SomeGlobal becomes a global available to app.ts (and no triple-slash used). Do you agree this is also a problem?

@mhegazy
Copy link
Contributor

mhegazy commented Sep 17, 2015

this should be partially handled by #4738. the full discussion is in #4665

@mhegazy mhegazy closed this as completed Sep 17, 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
None yet
Projects
None yet
Development

No branches or pull requests

4 participants