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

feat: Convert to async API #489

Merged
merged 1 commit into from Oct 9, 2019
Merged

feat: Convert to async API #489

merged 1 commit into from Oct 9, 2019

Conversation

coreyfarrell
Copy link
Member

BREAKING CHANGE: MapStore#transformCoverage is now async and returns a
the coverage data only. The sourceFinder method is now async and
provided directly on the MapStore instance.


CC @SimenB I know you use this API in jest, any objections? We're not yet upgrading to source-map@0.7.x but this will make that upgrade possible without further changes to the public API. Moving the sourceFinder method from the result of transformCoverage to the class itself is not strictly needed but just seems correct.

BREAKING CHANGE: MapStore#transformCoverage is now async and returns a
the coverage data only.  The `sourceFinder` method is now async and
provided directly on the `MapStore` instance.
@SimenB
Copy link
Member

SimenB commented Oct 8, 2019

CC @SimenB I know you use this API in jest, any objections?

Should be fine, the only things jest needs to be sync is source transformation and source maps for errors. Coverage calculation is perfectly fine async 👍

@coreyfarrell
Copy link
Member Author

What do you mean source maps for errors? You mean production of source maps during the transformation or something else?

@SimenB
Copy link
Member

SimenB commented Oct 8, 2019

Nothing to do with istanbul, I just mentioned which parts related to sourcemaps Jest needs to have sync. (when running tests we might get errors that we wanna map through sourcemaps)

@coreyfarrell coreyfarrell merged commit f8ebbc9 into istanbuljs:master Oct 9, 2019
@coreyfarrell coreyfarrell deleted the async-source-map branch October 9, 2019 01:02
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

2 participants