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

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 jvilk force-pushed the jvilk:typings-fix branch from 4dc14ce to 7cc0c9d Feb 13, 2018
@jvilk

This comment has been minimized.

Copy link
Contributor Author

commented Feb 13, 2018

(Updated with package.json change.)

Copy link
Member

left a comment

LGTM! Thanks @jvilk !

@fitzgen fitzgen merged commit c73baa5 into mozilla:master Feb 14, 2018
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@alexeagle

This comment has been minimized.

Copy link

commented Feb 19, 2018

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

This comment has been minimized.

Copy link

commented Feb 26, 2018

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.

@renovate renovate bot referenced this pull request Sep 23, 2018
0 of 1 task complete
@renovate renovate bot referenced this pull request Dec 12, 2018
0 of 1 task complete
@renovate renovate bot referenced this pull request Jan 9, 2019
0 of 1 task complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.