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

Update source-map TypeScript typings to 0.7.0 #321

Merged
merged 1 commit into from Feb 14, 2018

Conversation

jvilk
Copy link
Contributor

@jvilk jvilk commented Feb 13, 2018

First off: Congrats on the excellent 0.7.0 release! I look forward to measuring the performance impact on my memory leak debugger, BLeak.

The TypeScript typings contained within this repository are incorrect for the current version posted to NPM, which has breaking changes from 0.5.0. As a result, TypeScript code is unable to properly interoperate with the latest version of the library.

I have updated the typings to make the SourceMapConsumer constructors return a Promise and to add typings for the new destroy() and with methods.

I have also added a types field to package.json, so users of source-map can import from this module without requiring the @types/source-map metapackage.

If you accept and merge this pull request, please publish a new version of source-map to NPM with these changes so TypeScript code can interoperate with 0.7.0! Otherwise, TypeScript code will continue to use the invalid typings file published with the NPM module.

@jvilk
Copy link
Contributor Author

jvilk commented Feb 13, 2018

(Updated with package.json change.)

Copy link
Contributor

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks @jvilk !

@fitzgen fitzgen merged commit c73baa5 into mozilla:master Feb 14, 2018
@alexeagle
Copy link

Is it actually an advantage that the typings are included in the source-map package now? Unless you add some tests that the typings are correct (we have an example in Angular if you want) - then it's just an unmaintained extra file, and we would be better off having @types/source-map maintained by the TypeScript community...

@panuhorsmalahti
Copy link

There are some advantages if they're included in the package. The user of the library doesn't need to separately a) check if @types/source-map exists, b) install it and c) worry about version differences between the types and the code. However, you do bring up a valid point: the TypeScript definitions need to be maintained.

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.

None yet

4 participants