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

Built-in support for UMD module definitions #7125

Closed
RyanCavanaugh opened this Issue Feb 18, 2016 · 64 comments

Comments

Projects
None yet
@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Feb 18, 2016

Edit 2/29: Update proposal based on design changes; moved 'Solution' to top since most people viewing this are familiar with the problems


Solution

Support a "native UMD" declaration form that allows for a global var to be of the type of a module.

A global module export declaration has the syntax:

export as namespace id;

where id is any Identifier.

This is only legal as a top-level declaration a .d.ts file containing other top-level export declarations (e.g. export function, export var, export =, etc.). Multiple of these declarations may appear in the same file so long as they supply different identifiers.

When the containing file is imported through import syntax, the declaration has no effect.

When the containing file is added to the compilation via a /// <reference directive or by being a top-level file provided to the compiler (e.g. on the commandline, or as part of tsconfig.json's files list), the supplied identifier[s] are added to the global scope of the program. The type of these identifiers is the type of the module object of the file in which they were declared.

These declarations may engage in module merging, though this should probably be discouraged somehow.

Example

my-lib.d.ts

export function doThing(): string;
export function doTheOtherThing(): void;

export as namespace myLib;

globalConsumer.ts

/// <reference path="my-lib.d.ts" />

myLib.doThing();

importConsumer.ts

import * as m from './myLib';
m.doTheOtherThing();

Problem

Many definition flies are written like this:

declare var Promise: {
  // some stuff here
}
declare module 'bluebird' {
    export = Promise;
}

Symptoms

This is bad for three reasons:

  1. We can't find this file from normal module resolution, so it has to get into the compilation context through a 'reference' mechanism
  2. This pattern can't handle the case where you have two versions of 'bluebird' because there is no path with which to disambiguate
  3. People who are using the module version still get global scope pollution and might accidently use the global names when they meant to use an import name

Root Cause

The reason people do this (define globals, then export = from an ambient module declaration) is that the *reverse * (define a module, then declare a var with that module's type) is impossible.

@mhegazy

This comment has been minimized.

Copy link
Contributor

mhegazy commented Feb 18, 2016

Syntax bikeshedding is fun :), some more options:

namespace Promise from 'bluebird`;

namespace Promise = require("bluebird");

declare var Promise: typeof require("bluebird");
@DanielRosenwasser

This comment has been minimized.

Copy link
Member

DanielRosenwasser commented Feb 18, 2016

Syntax bikeshedding is great, but it depends a lot on what the intended semantics are.

It's important to note that the general syntax around imports has been frustratingly confusing, and it needs to be obvious what is happening in each instance. For example, I assumed the following from each of the above:

Syntax Semantics
namespace Promise from 'bluebird' Gives Promise the meaning of the default export from bluebird.
import global Promise from 'bluebird' Gives Promise the meaning of the default export from bluebird.
namespace Promise = require("bluebird") Gives Promise the general shape of the export= if present, or the whole module otherwise.
declare var Promise: typeof require("bluebird") Gives Promise the general shape of the export= if present, or the whole module otherwise.
@RyanCavanaugh

This comment has been minimized.

Copy link
Member

RyanCavanaugh commented Feb 18, 2016

⭐️ 🚲 🏠 ⭐️

My first intuition was import since that's the only keyword we have which acquires all meanings of its target, but obviously it's very confusing to have exactly one form of import ... 'modulename' which turns its containing file into a module. I'd like to avoid it if possible.

global is a mixed bag but I'm leaning toward it. It reinforces that this identifier goes into the global scope despite any connotations you might have about seeing a module name in the declaration.

It's also going to be an ambient-only thing, which makes me think we need declare as part of the construct so people know it doesn't have any runtime meaning.

I would like to avoid require since we are promoting ES6 import syntax in all other forms.

Putting all that together I would consider

declare global Promise from 'bluebird';

to be the most indicative of our intent.

@yortus

This comment has been minimized.

Copy link
Contributor

yortus commented Feb 18, 2016

Maybe there was some more detailed team discussion preceding this issue, but I can't quite grasp exactly what's being proposed here from the issue text. Is this related to #7015 (comment) (export = problem)?
Or is it solving something else?

It seems like the proposed solution still involves creating an ambient external module and an ambient global name, but in the opposite order. Why would import sites want to create an ambient global when they import bluebird? Can someone clarify?

@RyanCavanaugh

This comment has been minimized.

Copy link
Member

RyanCavanaugh commented Feb 18, 2016

The root root cause is that we're trying to design a good type acquisition strategy. That sounds simple enough, but when you start diving into the scenarios that are going to come up in the future when .d.ts files start following semver breaks, things get hairy pretty quickly.

The general idea here is to allow file structures like this:

mylib.d.ts

export function DoSomething() { }

mylib-global.d.ts

declare global myLib from './mylib';

The root problem here is that you might be using myLib v1 and myLib v2 in the same compilation context (because you depend on libraries A and B which, respectively, import the v1 and v2 definitions of myLib). Today, this is going to get you a one-way ticket to "Duplicate identifier" hell if there are separate definition files for v1 and v2.

But if you only consume myLib (directly or indirectly) via module imports, there's actually no conflict, because myLib-global.d.ts never enters the compilation context. By having the .d.ts files be "proper" modules (top-level exports, not declare module "mylib" {) it becomes possible to resolve the conflict via file paths and everything Just Works.

On the other hand, if two libraries claim they're both consuming myLib from a global reference, there really is a problem and the user is going to have to resolve that conflict by deciding which global is actually loaded at runtime (you'll see another issue sometime soon on how we intend to solve that problem).

@yortus

This comment has been minimized.

Copy link
Contributor

yortus commented Feb 18, 2016

Aha got it, thanks. This will be very useful.

So with this fix in place, would the idea be to rewrite things like bluebird.d.ts, which still have just a single top-level export, like the following?

// File: bluebird.d.ts

declare var Promise: PromiseConstructor;
//...
interface Promise<T> {
    then<U>...
    //...
}
declare namespace Promise {
    export interface CancellationError...
    export interface Resolver...
    //...
}
export = Promise;

... the main difference being there's no longer the ambient declare module "bluebird" {...} in there?

@RyanCavanaugh

This comment has been minimized.

Copy link
Member

RyanCavanaugh commented Feb 18, 2016

Right, then include a separate bluebird-global.d.ts which contains simply (🚲 🏠)

declare global Promise from 'bluebird';

which you would /// <reference .... if you were using in a script (non-loader) context

@yortus

This comment has been minimized.

Copy link
Contributor

yortus commented Feb 18, 2016

Brilliant! +1 :) This provides much closer correspondence between what happens at compile-time and run-time.

Partly related, but do you know if the formulation I wrote for bluebird.d.ts above would support module augmentation now that #6742 has been merged? I've been trying with typescript@next but always run into the 'can't add top-level names' error.

If we get this declare global and also declaration merging for export= both fixed, then TypeScript will be able to model pluggable CommonJS/AMD libraries very well indeed, without the whole ambient globals problem.

@kitsonk

This comment has been minimized.

Copy link
Contributor

kitsonk commented Feb 18, 2016

Just being devil's advocate for the 🚲 🏠, why is the team staying away from non semantic directives like the triple-slash comments?

Or would this be a legitimate use case for design time decorators (#2900) with something like:

@@require('bluebird', Promise);
declare var Promise;

I guess what I want to challenge is that this (while 100% useful) is even more "erasable" than typical typing information.

@billti

This comment has been minimized.

Copy link
Member

billti commented Feb 18, 2016

In the spirit of UMD, I think it would be beneficial to be able to write the types for a library in one file regardless of how it is referenced, rather than needing two .d.ts files to declare its types depending on if it is used as a global or a module - especially as it's quite likely only one line needed in the majority of cases to declare that when used as a script, some global identifier has the same shape as the module has.

First, to be sure everyone is clear on terminology for this discussion (as I had to look it up 😄 ): An ambient module declaration is of the form declare module "somename" {...}, and a declaration module is a .d.ts file that contains top level export/import declarations (e.g. export declare var foo: number;).

Part of the reason nearly all type definitions for modules today are written as ambient module declarations, is that declaration modules need to live in a location where module resolution would be able locate them. If we want to encourage .d.ts files for modules to be written as declaration modules, then we need a resolution mechanism whereby if my code says import {foo} from 'bar', then it can locate the .d.ts for bar, which quite possibly won't be shipped with bar or reside in myapp/node_modules/bar/. Let's assume we solve this, and declaration files for modules start to be authored as declaration modules and TypeScript locates them correctly. Awesome! That's a few problems solved.

Now two challenges: How could I also declare the types in this same file when referenced as a script (global), and how do I reference it in my app.

For the first, I don't see any reason why the same syntax outlined above couldn't work, i.e. declare global var $ from... . If the declaration of $ should look the same as the declaration module it is contained within, then a special form such as declare global var $ from this or declare global var $ from ".". I think it's also important to allow the current ambient declarations with the global modifier, as when used as a script it may introduce additional artifacts the module doesn't (e.g. could still write declare global function jquery ...).

To reference this as a global (assuming for a module you don't need to reference it, module resolution finds it when your app imports it), you could either reference the file directly as we do today (i.e. /// <reference path='./where/did/this/get/downloaded/to/jquery.d.ts'/>), or support a new form such as <reference library='jquery'/>, which would do the file resolution as for module resolution when looking up an import of jquery, but would adds it globals to the compilation.

Thus a canonical declaration may look something like:

// .d.ts file for the 'breakfast' library
export interface Eggs { /* ... */ }
export function sausages(): string;
export var bacon: Eggs;

declare global var breakfast from this;

To use it in a module I could just write the below (and the module .d.ts would resolve)

import * as foo from 'breakfast'
foo.sausages();
// etc...

Or to use it as a global script I could write something like:

/// <reference library="breakfast"/>
console.log(breakfast.bacon);
// etc..

Thoughts?

@RyanCavanaugh

This comment has been minimized.

Copy link
Member

RyanCavanaugh commented Feb 18, 2016

We currently only have one bit per file: Is this file part of your compilation, or not? If we make this a two-bit system:

  • Was this file imported?
  • Was this file referenced?

... now we have to figure out questions like:

  • If a UMD file is in tsconfig.json's files list, do we include its global?
  • If a UMD file is passed on the commandline, do we include its global?
  • If a user opens two loose files in VS, do we include the globals from UMD files?
  • Someone starts with a reference directive and an import to the same UMD file in file A, and uses the global export in file B. They realize that the file is in the compilation already, so they remove the reference directive, and now file B has errors. How do we explain this coherently?

This is a lot of complexity vs this guidance:

  • If you want to use the module version, use import. An import of a script file fails.
  • If you want to use the global version, use reference library =.... A reference to a module file fails.
@billti

This comment has been minimized.

Copy link
Member

billti commented Feb 18, 2016

Not sure I see this as overly complex. To your questions:

If a UMD file is in tsconfig.json's files list, do we include its global?

Yes. If the .d.ts file was included by a means besides module resolution, then the globals are declared.

If a UMD file is passed on the commandline, do we include its global?

Yes. If the .d.ts file was included by a means besides module resolution, then the globals are declared.

If a user opens two loose files in VS, do we include the globals from UMD files?

Not sure I understand. Are these files the .d.ts files in question? One of them? Are they referencing or importing said .d.ts files somehow? Can you give me more details on the scenario you had in mind as problematic?

Someone starts with a reference directive and an import to the same UMD file in file A, and uses the global export in file B. They realize that the file is in the compilation already, so they remove the reference directive, and now file B has errors. How do we explain this coherently?

How is this different to today where if I remove a reference to a .d.ts file I need, then the declaration disappears and now files using it have errors? With the proposed system the import would resolve the module typing, but they'd need a reference to the .global.d.ts to get the global identifier, so doesn't the same scenario/error remain if they remove the library reference?

This is a lot of complexity vs this guidance...

There may be some work on the compiler side (there is for any of this), but I'm trying to simplify the experience for the end user and type definition author, and maintain the "one JS library = one .d.ts file" symmetry we generally have today. The guidance of "import a module and the typings just work, reference a typing to get the globals" remains the same in either, the guidance of "download one definition file that matches the library name and reference it to add global typings" seems simpler than "download these two definition files for each library and reference the one that has global in the name to get the global typings"

Maybe this is bike-shedding, as either seems workable and better than what we have now. I'm just looking for the optimal simplicity for TypeScript users in what has been a confusing space to date.

@RyanCavanaugh

This comment has been minimized.

Copy link
Member

RyanCavanaugh commented Feb 18, 2016

I think your approach is workable; I've implemented a prototype and it's not as complex as I expected.

https://github.com/RyanCavanaugh/TypeScript/tree/umd

Maybe pull it down and try it out

@RyanCavanaugh

This comment has been minimized.

Copy link
Member

RyanCavanaugh commented Feb 29, 2016

Slogged; we like export as namespace $; as a better syntax

@RyanCavanaugh

This comment has been minimized.

Copy link
Member

RyanCavanaugh commented Mar 1, 2016

Facts:

  • May not appear in ambient external modules
  • May not appear in implementation files
  • May not have modifiers
  • May not appear in non-module files

Questions:

  • May cause 'duplicate identifier' errors?

@RyanCavanaugh RyanCavanaugh changed the title Support UMD module definitions Built-in support for UMD module definitions Mar 1, 2016

@kitsonk

This comment has been minimized.

Copy link
Contributor

kitsonk commented Mar 1, 2016

👍 for the syntax

@bytenik

This comment has been minimized.

Copy link

bytenik commented Jul 11, 2016

Is this not included in 2.0 beta?

@mhegazy

This comment has been minimized.

Copy link
Contributor

mhegazy commented Jul 11, 2016

@bytenik

This comment has been minimized.

Copy link

bytenik commented Jul 11, 2016

Does this not support legacy typings without the new export syntax? i.e. the SignalR typings file in @types/signalr

@mhegazy

This comment has been minimized.

Copy link
Contributor

mhegazy commented Jul 11, 2016

the @types definitions are pushed from definitelyTyped. we have manually updated some of these packages. the sources are at https://github.com/DefinitelyTyped/DefinitelyTyped/blob/types-2.0/signalr/index.d.ts, feel free to send a PR for any changes and we will get these in.

@tomwanzek

This comment has been minimized.

Copy link

tomwanzek commented Jul 12, 2016

I have a quick question regarding the following quote at the very end of the Solution summary by @RyanCavanaugh :

These declarations may engage in module merging, though this should probably be discouraged somehow.

I have been drawing up new typescript definitions for Mike Bostock's popular, newly modularized version 4 of the D3 data visualization tools. D3 is now split up into several modules, e. g. d3-selection, d3-transition, d3-shape etc...

There is also a standard pre-built bundle of the 'core' modules, which is provided as a single module d3.
The modules are structured as UMD modules for bundled/unbundled use in vanilla script as well as module import use cases.

In writing the definitions, I came across the following issue related to their UMD character as. For the vanilla script scenario:

  1. if the standard pre-built bundle of modules is loaded from the single bundle file, it exposes a d3 global with the objects exposed by each of the modules which feed the standard bundle
  2. if the scripts of the d3 modules are individually included (i.e. unbundled), each of them exports to/extends the same d3 global. Mike's intent being ease of code reuse for D3 users in the vanilla scenario.

Ad 1): Creating a bundle definition with UMD characteristics for the default bundle d3 is as simple as re-exporting the relevant 'core' modules and adding the export as namespace d3; for the global.

Ad 2): I am running into the issue that, adding the export as namespace d3; to the individual modules, e.g. d3-selection, d3-transition etc., creates a duplicate identifier compile error for the d3 identifier. (Typescript v2.0.0.) (Note: there are no identifier conflicts between the objects exported from the individual modules)

Despite the aforementioned quote, I suspect this is expected compiler behavior? Is there a preferred way to accomplish the module merging into the global d3?

@mhegazy

This comment has been minimized.

Copy link
Contributor

mhegazy commented Jul 13, 2016

@tomwanzek it is kinda hard to see what is happening without looking at the code. is there a chance you can share your declarations and the errors you are getting. it would also be great if you open a new issue for this.

@unional

This comment has been minimized.

Copy link
Contributor

unional commented Jul 13, 2016

My gut tells me you need to do global augmentation for 2)

@tomwanzek

This comment has been minimized.

Copy link

tomwanzek commented Jul 13, 2016

Thanks for the quick response. @mhegazy I created a repo here to stage the D3 definitions while I am drafting them. I did not use my DefinitelyTyped fork to create a pull-request right away, because I am using this typing of callback function contexts, which was not yet available in typescript 1.8.10.

I intend to move them into DefinitelyTyped as soon as possible for those that have completed 'shape tests' of the definitions. And then incrementally as testing completes.

The repo itself carries an issue for the d3 global question raised above here. Representative definition files can be found here for e. g.:

Note, that these definitions currently do not individually have the export namespace as d3;, because of the mentioned issue. I added them locally and get the compile error.

There is a definition file for the 'bundled' d3 module here, which uses re-exports and has the global export.

I can open a new issue for typescript and cross-reference anew, do you have a preferred name to track it?

@bytenik

This comment has been minimized.

Copy link

bytenik commented Jul 13, 2016

I'm confused still. Did <reference types="x" /> not make it in? If so, how does one .d.ts going into DefinitelyTyped reference a different module's definition?

@RyanQuackenbush

This comment has been minimized.

Copy link

RyanQuackenbush commented Jul 19, 2016

@bytenik I've tested 2.0. and it seems to be working fine (using <reference types="x" />). The libraries I've installed via npm install @types/library_name use the same syntax.

@speigg

This comment has been minimized.

Copy link

speigg commented Aug 4, 2016

Nothing changes in modules. the way modules were working before should continue to work. the only change is for global. and ///<reference types=<> /> should not be used on modules. I would say it should be an error if you use///<reference types=<> /> to import a module.

@mhegazy Am a bit annoyed by this restriction right now. Getting a TS2686 error for referencing a UMD identifier (which is loaded globally) from within a module. I tried using types in tsconfig.json instead of ///<reference types=<> /> and that didn't help either.

@RyanCavanaugh

This comment has been minimized.

Copy link
Member

RyanCavanaugh commented Aug 5, 2016

@speigg very curious how you're getting into this state -- you're loading some libraries globally, and some through a module loader?

@speigg

This comment has been minimized.

Copy link

speigg commented Aug 5, 2016

you're loading some libraries globally, and some through a module loader?

@RyanCavanaugh Yes, exactly.

@RyanCavanaugh

This comment has been minimized.

Copy link
Member

RyanCavanaugh commented Aug 5, 2016

@speigg logged #10178

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