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

Allow importing of all declared types #736

Closed
wants to merge 2 commits into from

Conversation

whatisaphone
Copy link

There's been a regression in the TypeScript definitions. In version 3.2.0, I was able to import raven's types like so:

import Raven, { RavenOptions } from 'raven-js';

// further down...
export function captureMessage(msg: string, options?: RavenOptions) {
    Raven.captureMessage(msg, augmentOptions(options));
}

function augmentOptions(options?: RavenOptions): RavenOptions {
    // ...
}

In 3.7.0, the types are no longer exported, so this is no longer possible. This change would make it possible again.

I didn't re-indent the file in order to keep the diff obvious, but obviously it would be good to indent everything inside the new module block.

@benvinegar
Copy link
Contributor

I didn't re-indent the file in order to keep the diff obvious, but obviously it would be good to indent everything inside the new module block.

Indenting is fine. I just need to verify that this all works fine with an Angular 2 quickstart application and I'll go ahead and merge.

@MaxBittker
Copy link
Contributor

hi @johnsoft - I tried your patch with typescript 2.0.3 and had the error
a 'declare' modifier is required for a top level declaration in a .d.ts file at line 9
do you know what this is about?

@whatisaphone
Copy link
Author

Hey, sorry for the delay, we've been busy here.

Either I made a typo, or that was a difference between typescript 1.x and 2.0. I added the modifier it was complaining about, and it now works fine with [email protected].

@LucaVazz
Copy link
Contributor

LucaVazz commented Jul 7, 2017

With #977 this could be closed, right @johnsoft?

@whatisaphone
Copy link
Author

@LucaVazz Yep, this can be closed

@MaxBittker
Copy link
Contributor

👍 thanks ya'll

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.

5 participants