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: function to cleanup allocated resources after usage #161

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions README.md
Expand Up @@ -32,6 +32,10 @@ converter.applyCoverage([
// output coverage information in a form that can
// be consumed by Istanbul.
console.info(JSON.stringify(converter.toIstanbul()))

// cleanup resources allocated in "load" (i.e. by the source-map dependency),
// the converter may not be used anymore afterwards
converter.destroy()
```

## Ignoring Uncovered Lines
Expand Down
1 change: 1 addition & 0 deletions index.d.ts
Expand Up @@ -15,6 +15,7 @@ declare type Sources =
}
declare class V8ToIstanbul {
load(): Promise<void>
destroy(): void
applyCoverage(blocks: ReadonlyArray<Profiler.FunctionCoverage>): void
toIstanbul(): CoverageMapData
}
Expand Down
6 changes: 6 additions & 0 deletions lib/v8-to-istanbul.js
Expand Up @@ -83,6 +83,12 @@ module.exports = class V8ToIstanbul {
}
}

destroy () {
if (this.sourceMap) {
this.sourceMap.destroy()
SimenB marked this conversation as resolved.
Show resolved Hide resolved
}
}

_resolveSource (rawSourceMap, sourcePath) {
if (sourcePath.startsWith('file://')) {
return fileURLToPath(sourcePath)
Expand Down
16 changes: 16 additions & 0 deletions test/v8-to-istanbul.js
Expand Up @@ -22,6 +22,8 @@ describe('V8ToIstanbul', async () => {
v8ToIstanbul.covSources[0].source.lines.length.should.equal(48)
v8ToIstanbul.covSources.length.should.equal(1)
v8ToIstanbul.wrapperLength.should.equal(0) // common-js header.

v8ToIstanbul.destroy()
})

it('handles ESM style paths', async () => {
Expand All @@ -33,6 +35,8 @@ describe('V8ToIstanbul', async () => {
v8ToIstanbul.covSources[0].source.lines.length.should.equal(48)
v8ToIstanbul.covSources.length.should.equal(1)
v8ToIstanbul.wrapperLength.should.equal(0) // ESM header.

v8ToIstanbul.destroy()
})

it('handles source maps with sourceRoot', async () => {
Expand Down Expand Up @@ -60,6 +64,8 @@ ${'//'}${'#'} sourceMappingURL=data:application/json;base64,${base64Sourcemap}
await v8ToIstanbul.load()

v8ToIstanbul.path.should.equal(absoluteSourceFilePath)

v8ToIstanbul.destroy()
})

it('handles sourceContent', async () => {
Expand Down Expand Up @@ -91,6 +97,8 @@ ${'//'}${'#'} sourceMappingURL=data:application/json;base64,${base64Sourcemap}
// if the source is transpiled and since we didn't inline the source map into the transpiled source file
// that means it was bale to access the content via the provided sources object
v8ToIstanbul.sourceTranspiled.should.not.be.undefined()

v8ToIstanbul.destroy()
})

it('should clamp line source column >= 0', async () => {
Expand Down Expand Up @@ -121,6 +129,8 @@ ${'//'}${'#'} sourceMappingURL=data:application/json;base64,${base64Sourcemap}
endOffset: matchedNewLineChar + 10
}]
}])

v8ToIstanbul.destroy()
})

it('should exclude files when passing excludePath', async () => {
Expand All @@ -139,6 +149,8 @@ ${'//'}${'#'} sourceMappingURL=data:application/json;base64,${base64Sourcemap}
}]
}])
Object.keys(v8ToIstanbul.toIstanbul()).should.eql(['/src/index.ts', '/src/utils.ts'].map(path.normalize))

v8ToIstanbul.destroy()
})
})

Expand All @@ -157,6 +169,7 @@ ${'//'}${'#'} sourceMappingURL=data:application/json;base64,${base64Sourcemap}
0
)
await v8ToIstanbul.load()
v8ToIstanbul.destroy()
})

it('should handle relative sourceRoots correctly', async () => {
Expand All @@ -166,6 +179,7 @@ ${'//'}${'#'} sourceMappingURL=data:application/json;base64,${base64Sourcemap}
)
await v8ToIstanbul.load()
assert(v8ToIstanbul.path.includes(path.normalize('v8-to-istanbul/test/fixtures/one-up/relative-source-root.js')))
v8ToIstanbul.destroy()
})

it('should handles source maps with multiple sources', async () => {
Expand All @@ -177,6 +191,8 @@ ${'//'}${'#'} sourceMappingURL=data:application/json;base64,${base64Sourcemap}

v8ToIstanbul.covSources.length.should.equal(3)
Object.keys(v8ToIstanbul.toIstanbul()).should.eql(['/webpack/bootstrap', '/src/index.ts', '/src/utils.ts'].map(path.normalize))

v8ToIstanbul.destroy()
Copy link
Member

Choose a reason for hiding this comment

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

having a test that verifies that the source map has been destroyed would be good, perhaps you could follow @SimenB's recommendation of setting it equal to undefined, and then just assert this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added a new test which covers this

})
})

Expand Down