-
-
Notifications
You must be signed in to change notification settings - Fork 39
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
feat: function to cleanup allocated resources after usage #161
Conversation
I've run into this in practice when using the package together with https://github.com/microsoft/playwright, similar to the suggestion in https://playwright.dev/docs/api/class-coverage. When running the collection with a larger number of tests and therefore creating a new instance for each test, the converter fails due to an error message which matches the ones in the linked mozilla/source-map issue, which can be clearly attributed to the missing |
@@ -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() |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
@ssams thanks for the great contribution, I left one nit. |
I've set the source map to |
@bcoe can you ping me when you release this so I can update in Jest to use it? 🙂 |
Ah #163, never mind 😀 |
Adds a
destroy
function as counterpart to the existingload
to cleanup allocated resources after usage, especially memory allocated by theSourceMapConsumer
from the source-map dependency.Currently the converter creates a new
SourceMapConsumer
during the load operation, however this gets never destroyed properly. Documentation states that "When the SourceMapConsumer will no longer be used anymore, you must call its destroy method." (https://github.com/mozilla/source-map/blob/58819f09018d56ef84dc41ba9c93f554e0645169/README.md#new-sourcemapconsumerrawsourcemap). Failing to cleanup the consumer can trigger errors such as mozilla/source-map/issues/412.