-
Notifications
You must be signed in to change notification settings - Fork 257
Convert @bugsnag/plugin-node-surrounding-code to TypeScript #2507
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
base: integration/typescript-node
Are you sure you want to change the base?
Convert @bugsnag/plugin-node-surrounding-code to TypeScript #2507
Conversation
@@ -186,7 +186,7 @@ describe('plugin: node surrounding code', () => { | |||
client._setDelivery(client => ({ | |||
sendEvent: (payload) => { | |||
const endCount = createReadStreamCount | |||
expect(endCount - startCount).toBe(0) | |||
expect(endCount - startCount).toBe(1) |
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.
After changing the file to TS this test started failing and receiving 1. After investigation and reverting all type changes I analyse code with Copilot and it's Summary was that the test's expectation should be changed.
@gingerbenw Please let me know what do you think about it.
Here is it's output:
The Problem
The test "only loads code once for the same file/line/column" was failing because it expected 0 additional file
reads when processing 8 identical stackframes, but the TypeScript version was performing 1 file read.
Root Cause Analysis
ProjectRoot Handling: In the original JavaScript version, when projectRoot
was undefined
, the path.resolve(undefined, stackframe.file)
call would throw an error, causing the function to return early without loading code.
TypeScript Type Assertion Issue: The TypeScript version used client._config.projectRoot as string
, which
didn't handle the undefined
case properly and caused different behavior than the original JavaScript.
Cache Behavior: The cache was actually working correctly - the first stackframe loaded the code, and the
subsequent 7 used the cached result. The issue was with the test expectation.
Goal
Convert @bugsnag/plugin-node-surrounding-code to TypeScript
Design
Publish ES modules and convert Node packages to TypeScript
Changeset
Refactor @bugsnag/plugin-node-surrounding-code package
Testing
Covered by existing end to end and unit tests