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

Upgrades to TypeScript 1.3. Fixes Bug 187. #198

Closed
wants to merge 3 commits into from
Closed

Upgrades to TypeScript 1.3. Fixes Bug 187. #198

wants to merge 3 commits into from

Conversation

mtraynham
Copy link

In lieu of microsoft/TypeScript#512, this upgrades to TypeScript 1.3. Seems there were pretty minimal changes (lot of white space removed) to the transpiled code.

One thing to point out. I didn't check in the tasks/**/*.js files. I figured those were generated, and can I recommend you just add those as a build step before testing, removing them from the source tree. They can be regenerated by grunt build, but I think I'm preaching to the choir.
I did however check in the tests/{transform,simple}/**/*.ts files, but I would recommend those be removed from the source tree as well.

This should also be a fix for Bug #187. Using the old method of:

///ts:export Foo
import Foo_file = require('./Foo');
export var Foo = Foo_file;

causes definition file import issues in dependent modules, especially when using Classes as parameter types. Note, the example below also used dts-bundle:

**Project A generated project-a.d.ts**

    declare module 'project-a' {
        import Foo_file = require('__project-a/Foo');
        export declare var Foo: typeof Foo_file;
    }

    declare module '__project-a/Foo' {
        class Foo {
             name: string;
        }
    }

**Project B/Bar.ts**

    import ProjectA = require('project-a');

    class Bar {
        getData(aInstance: ProjectA.Foo) => aInstance.name; // Module ''project-a'' has no exported member 'Foo'
        getData(aInstance: typeof ProjectA.Foo) => aInstance.name; // Property 'name' does not exist on type 'typeof Foo'
    }

@mtraynham
Copy link
Author

In hindsight, I might just check in the javascript files, or update the npm (or grunt) build to run that first. Build seems to fail if I don't. Up to you guys.

@mtraynham
Copy link
Author

Looks like @sebas2day had the same idea with #197. I would still recommend an upgrade to TS 1.3 though.

@mtraynham
Copy link
Author

Since 1.4 is out, looks like you guys are upgrading that way. I'll go ahead and close this.

@mtraynham mtraynham closed this Feb 4, 2015
@mtraynham mtraynham deleted the bug_187 branch February 4, 2015 00:14
@nycdotnet
Copy link
Contributor

Hey there @mtraynham - thanks for the pull request. I am very sorry that I just didn't have the time to look it over in the past month or so.

@mtraynham
Copy link
Author

Hey @nycdotnet. Quite alright, I've been using my fork for the most part, so no issue there. I saw that you guy's were planning an upgrade to 1.4, but I also noticed something weirdly wrong with my pull request. So I decided to close this out and revert back to 2.0.1. Unfortunately it doesn't totally work for me either.

Personally, I'm going to chalk this up to my own confusion.

Maybe you can help me though... I'm working on a library project which is split across multiple files. What is the proper way to export import multi-file external modules from an index.ts.

The example below shows what I mean. When developing ambient external modules, a "file" module can be represented as a class using the export = functionality. But after the dts-bundle this seems to no longer apply. Because the class is export =, I am not really able to new these classes in downstream projects. Although, it does seem that the class is basically an object instance.

After I made my change, things started compiling better downstream... But i'm not entirely sure that path is correct either.

So maybe you can give me some direction.

As an example:

a.ts:

class A {
    public foo: string;
}
export = A;

Standard version:

index.ts:

export import A = require('a');

index.d.ts:

export import A = require('a');

Grunt-ts version

index.ts

///ts:export A
import A_file = require('a');
export var A = A_file;

index.d.ts

///ts:export A
import A_file = require('a');
export var A = typeof A_file;

Output of dts-bundle

Standard version

index.d.ts

declare module 'module/A' {
    class A {
        public foo: string;
    }
    export = A;
}
declare module 'module' {
    export import = require('module/A');
}

Grunt-ts version:

index.d.ts

declare module 'module/A' {
    class A {
        public foo: string;
    }
    export = A;
}
declare module 'module' {
    import A_file = require('module/A');
    export var A = typeof A;
}

@nycdotnet
Copy link
Contributor

You're not confused - or at least you are confused, but you're in good company. This is one of the major outstanding pain points with TypeScript since pretty much day 1. I must be honest that I have not yet seen a good way to do multi-file libraries in TypeScript when using external modules that allows a separate project (such as a UI) to use the typing from the library in an effortless manner. I would love to be wrong.

I see that you already posted here microsoft/TypeScript#17 (notice the low issue number). Another good thread is here: microsoft/TypeScript#1753 .

I've been lucky enough to be able to avoid external modules altogether and just use TypeScript internal modules for the browser-based projects I work on. When considering the performance drawbacks of issuing many HTTP requests, a large minified JS file that exposes only a single global variable is often the best solution.

I do have an idea to upgrade grunt-ts to be able to generate UMD files from internal modules, but that's for another day.

@mtraynham
Copy link
Author

:P I have to say, I think I've tried 14 different ways to do things, always looking for the best one (tutorials are slim to none). Sometimes I turn up empty-handed, sometimes not, but my hopes are still pretty high.

I think I might be on to one thing though, which the author of https://github.com/SitePen/dts-generator alluded to. Maybe the single point of entry should be avoided for multi-file libraries. Instead of requiring the main module, you just require by path and avoid the large list of export import's. I think it could actually be achieved using webpack which has multi-entry support. Although, this does make the main module a little strange to use...

If I don't come up empty-handed, I'll be sure to drop a note somewhere.

This PR though, I'd say overlook it.

UMD files from internal modules :) Interesting. Thanks for the feedback and hope the 1.4.x efforts go well, should be a good upgrade!

@nycdotnet
Copy link
Contributor

Thanks very much. If you figure out a good way to do it, please do let me know. I'll scream it from the mountaintops.

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

Successfully merging this pull request may close these issues.

2 participants