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

fix: Purge source-map cache before reporting if cache is disabled. #1080

Merged
merged 2 commits into from Apr 24, 2019

Conversation

@coreyfarrell
Copy link
Member

commented Apr 21, 2019

@bcoe this (combined with istanbuljs/istanbuljs#375) seems to fix #1049 (and likely others like it).

@bcoe

This comment has been minimized.

Copy link
Member

commented Apr 21, 2019

@istanbuljs/core awesome work 👍 weirdly, npm link isn't working well for me to test my changes in istanbul-lib-source-map. might need to release a next release, just for testing purposes.

lib/source-maps.js Outdated Show resolved Hide resolved
function SourceMaps (opts) {
this.cache = opts.cache
this.cacheDirectory = opts.cacheDirectory
this.loadedMaps = {}
this._sourceMapCache = sourceMapCache
}

SourceMaps.prototype.purgeCache = function () {
sourceMapCache = libSourceMaps.createSourceMapStore()

This comment has been minimized.

Copy link
@bcoe

bcoe Apr 21, 2019

Member

I don't understand why we'd need to throw out source maps like this, might be worth digging into things a bit more, and figuring out how we end up with incompatible source-maps in the first place.

I'm not opposed to landing this as a patch as a temporary measure, but it feels like we're continuing to complicate our source-map and caching logic, which is already a mess. I think you're right that we just need to get way more test cases in the codebase.

This comment has been minimized.

Copy link
@coreyfarrell

coreyfarrell Apr 21, 2019

Author Member

Same reason we need to throw out transform results (they set cache: false).

Remove source-maps singleton.
This is unneeded since we always limit ourselves to a single NYC
instance.

@coreyfarrell coreyfarrell marked this pull request as ready for review Apr 21, 2019

@coreyfarrell

This comment has been minimized.

Copy link
Member Author

commented Apr 21, 2019

might need to release a next release, just for testing purposes

For istanbul-lib-source-maps I think definitely. Once we do that I think we should ping Simen from jest to ensure it doesn't cause regressions for them.

index.js Show resolved Hide resolved

@coreyfarrell coreyfarrell merged commit 3d9eaa4 into istanbuljs:master Apr 24, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls First build on sm-purge-cache at 96.848%
Details

@coreyfarrell coreyfarrell deleted the coreyfarrell:sm-purge-cache branch May 2, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.