Skip to content
This repository was archived by the owner on Jan 5, 2024. It is now read-only.

Reduce duplicated effort for module+global/script typings #402

Open
unional opened this issue Apr 4, 2016 · 28 comments
Open

Reduce duplicated effort for module+global/script typings #402

unional opened this issue Apr 4, 2016 · 28 comments

Comments

@unional
Copy link
Member

unional commented Apr 4, 2016

Currently when we create typings that can be consumed as module AND global/script, we need to write it twice.

e.g.:
https://github.com/typed-contrib/knockout/blob/master/index.d.ts
https://github.com/typed-contrib/knockout/blob/master/global/index.d.ts
and
https://github.com/typed-typings/typed-iscroll/blob/master/iscroll.d.ts
https://github.com/typed-typings/typed-iscroll/blob/master/iscroll/iscroll.d.ts

This is due to TypeScript does not allow this:

declare namespace abc {
  import main = require('./abcModule');
  export = main;

or similar construct.

Is there an issue opened on https://github.com/Microsoft/TypeScript can be referenced and tracked here?

@blakeembrey
Copy link
Member

Doesn't TypeScript 2.0 UMD support fix this specific issue?

@unional
Copy link
Member Author

unional commented Apr 4, 2016

microsoft/TypeScript#7125

Yeah, just need to refresh my memory and connecting the dots.
Can't wait for it to come out.

@unional
Copy link
Member Author

unional commented Apr 4, 2016

Workaround by @basarat
microsoft/TypeScript#7352 (comment)

UPDATE: However, I think this won't work if there are multiple references to the typings. e.g. you and one of your dependencies both reference it.

@johnnyreilly
Copy link
Member

@unional, thanks for the link to the workaround by @basarat. I'm afraid I don't quite follow how this approach could be applied to a type definition. If it's not too much trouble, would you be able to illustrate how this could be applied in the case of say iScroll or Knockout? I'd really like to understand this better.

@unional
Copy link
Member Author

unional commented Apr 7, 2016

I need to build an example to test the statement that I think it might not work.

The workaround is exposing them through two files, global.d.ts and globalAugment.d.ts and add them to typings.json/files

@basarat
Copy link
Member

basarat commented Apr 7, 2016

@johnnyreilly

Modules stay modules

Basically if a file is declared as a module, there is no way to move it into the global namespace without thinking too hard:

declare module "foo" {
    export var foo: any;
}

Any attempt to use this foo module would need an import:

import * as foo from "foo";

That would essentially make the file a module and therefore cannot pollute the global namespace.

Hack to move it into the global

The one linked here : microsoft/TypeScript#7352 (comment) works by pollute the global namespace seperately:

interface My {    // To introduce the name `My` to the global namespace
}

And then augment it from the module:

import foo = require('foo');
declare global {
    interface My {
        user: typeof foo;
    }
}

etc.

Note this is a very bad workaround for decreasing code duplication as we need two files (global.d.ts and my.d.ts) to put a module into the global declaration space.

Long term fix

UMD module declarations : microsoft/TypeScript#7125 now allow you to create a declaration file that is a module:

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

export as namespace myLib;

That can be imported as a module:

import * as foo from "./foo";

or can be imported globally:

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

myLib.doThing();

So we actually don't need any duplication effort on our part. The effort moves into the hands of the library consumer (either global or as a module). Hope that helps 🌹

@unional
Copy link
Member Author

unional commented Apr 7, 2016

@basarat, do you think that if it is being referenced multiple times, would it cause duplicate identifier error? That's what I want to test about. 🌹

@johnnyreilly
Copy link
Member

Thanks @basarat - I need to let this sink in a bit but I think I follow you.

So it's not really worth advising people to use the hack; just wait for UMD to ship. (Which looking at the talks from Build is probably only about 2 months or so) 🌷

@basarat
Copy link
Member

basarat commented Apr 7, 2016

do you think that if it is being referenced multiple times, would it cause duplicate identifier error

@unional are you asking about umd or hack?

@unional
Copy link
Member Author

unional commented Apr 7, 2016

Hack.

@basarat
Copy link
Member

basarat commented Apr 7, 2016

do you think that if it is being referenced multiple times, would it cause duplicate identifier error

@unional Not if its referenced multiple times pointing to the same file. However if its across different files its effectively the following and will result in an error:

declare global {    
    interface Array<T> {
        foo:number; // Duplicate ident error 
    }
}

declare global {
    interface Array<T> {
        foo:number; // Duplicate ident error
    }
}

@unional
Copy link
Member Author

unional commented Apr 7, 2016

Great. If that is the case, then it might be possible that we can start doing it today in typings. Since they are ambient, the dependencies aren't followed not inlined.

Therefore it should (crossing fingers) not cause problem at the end consumer.

Thanks for your great hack! 🌹

@unional
Copy link
Member Author

unional commented Apr 7, 2016

...nor inlined....
Phone keyboard doesn't have "nor " in the dictionary........

@unional
Copy link
Member Author

unional commented Apr 7, 2016

Just tried it. Having a bit of issue. ping @blakeembrey .
I've upload an example:
https://github.com/typed-typings/typed-iscroll/tree/master/global/iscroll

The problem I have is when I run typings bundle -o out --ambient on it, it fails with:

typings ERR! message Attempted to compile "iscroll" as an ambient module, but it looks like an external module.

typings ERR! cwd /Users/hwong/github/typed-typings/typed-iscroll/global/iscroll/out
typings ERR! system Darwin 14.5.0
typings ERR! command "/usr/local/bin/node" "/usr/local/bin/typings" "bundle" "-o" "out" "--ambient"
typings ERR! node -v v4.3.2
typings ERR! typings -v 0.7.12

typings ERR! If you need help, you may report this error at:
typings ERR!   <https://github.com/typings/typings/issues>

The message is right that indeed iscrollAugment.d.ts is an external module.....but.....

@blakeembrey
Copy link
Member

@unional What's the expected behaviour?

@unional
Copy link
Member Author

unional commented Apr 7, 2016

@blakeembrey how to make it work? 😛

When used, it needs to be in two separate files. Maybe index.d.ts and indexAugment.d.ts?

// main.d.ts
/// <reference path="main/ambient/iscroll/index.d.ts" />
/// <reference path="main/ambient/iscroll/indexAugment.d.ts" />

But on the other hand, it may not worthwhile because it could be too complicate to implement for a hack that will go away when 2.0 lands.

@blakeembrey
Copy link
Member

Don't you just bundle it without --ambient? It doesn't matter that it gets wrapped up, declare global is still good.

@unional
Copy link
Member Author

unional commented Apr 7, 2016

Without --ambient you get another error because iscroll.d.ts is ambient:

typings bundle -o out
typings ERR! message Unable to compile "iscroll", the typings are meant to be installed as ambient but attempted to be compiled as an external module

typings ERR! cwd /Users/hwong/github/typed-typings/typed-iscroll/global/iscroll
typings ERR! system Darwin 14.5.0
typings ERR! command "/usr/local/bin/node" "/usr/local/bin/typings" "bundle" "-o" "out"
typings ERR! node -v v4.3.2
typings ERR! typings -v 0.7.12

typings ERR! If you need help, you may report this error at:
typings ERR!   <https://github.com/typings/typings/issues>

@blakeembrey
Copy link
Member

Bundle the definition itself?

@unional
Copy link
Member Author

unional commented Apr 7, 2016

Bundle the definition itself?

What do you mean?

// typings.json
{
  "name": "iscroll",
  "main": "iscroll.d.ts",
  "ambient": true,
  "files": [
    "iscroll.d.ts",
    "iscrollAugment.d.ts"
  ],
  "version": "5.1.3",
  "homepage": "https://github.com/cubiq/iscroll",
  "devDependencies": {
    "blue-tape": "registry:npm/blue-tape#0.1.0+20160322235613"
  },
  "ambientDevDependencies": {
    "node": "registry:dt/node#4.0.0+20160319033040"
  }
}

@blakeembrey
Copy link
Member

I don't know what to say, can you provide a more isolated example? Looking at https://github.com/typed-typings/typed-iscroll/blob/master/global/iscroll/typings.json#L6-L7 you are trying to merge an ambient and external version which won't work. Why don't you make it only external versions?

@unional
Copy link
Member Author

unional commented Apr 7, 2016

Create an issue on typed-iscroll: typed-typings/npm-iscroll#4

@johnnyreilly
Copy link
Member

Maybe it's worth just waiting for 2.0? If it's only a couple of months away then this might be more trouble than it's worth. Also you might get people in the future looking at Typings with the hack in place and then thinking the hack is good practice.

@basarat
Copy link
Member

basarat commented Apr 8, 2016

Also you might get people in the future looking at Typings with the hack in place and then thinking the hack is good practice

💯 I am sure this wasn't intended as a part of declare global augmentation and we just found a way to abuse it. But then again I'm not the one feeling the pain of writing these duplicate definitions so whatever you chose 🌹 🙇

@unional
Copy link
Member Author

unional commented Apr 8, 2016

😄 maybe we can change topic on how to get typings ready for UMD. 😉

We still want to have a separate file for UMD. But now it will be in top-level import/export format. So we need to detect it is UMD and make it an exception to the current validation.

@basarat
Copy link
Member

basarat commented Apr 16, 2016

FWIW the hack is simpler now as you no longer need a separate file for introducing the name into the global namespace: microsoft/TypeScript#7161 (comment) 🌹 I still can't make the choice of what is the right way to do it though, and I leave that to your brilliant hands 🌹

@unional
Copy link
Member Author

unional commented May 25, 2016

Just think of an idea as I'm looking into fixing some issue on env-atom:

  • Create typings in module format, as in the source code.
  • Use typings bundle to create global typings
  • Use typings.json/files to add those bundled global typings.

@blakeembrey what do you think?

@blakeembrey
Copy link
Member

@unional That could work. I think if it works it's good. You may want to tweak the bundle command to omit dependencies just in case, but seems reasonable to do it this way and end up with just one type of bundle.

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

No branches or pull requests

4 participants