You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The starting point for this PR was analyzing how we create the Provider and how different pieces interact with each other in JS and Rust<>JS in general around the stack traces logic and tests.
This removes some of the dead or redundant code and refreshes the console.log signature lookup table generation script. If we decide to move it to Rust, we won't have to call JS anymore and move buffers around just to decode and execute the console.log formatting logic.
After moving the auxiliary script to TS I re-ran it to verify that it outputs the same code (incl. the changes from this PR).
…lass
This makes it clearer what the exact logic is and if additional state is
used, which will be helpful when porting this code to Rust as we don't
want to rely on an external console.log decoding logic in JS.
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
This PR includes no changesets
When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types
Checking my understanding here. This PR has two parts:
The most important part is turning ConsoleLogger into a bag of static methods, because (I guess) that will make porting it easier. Is this correct?
The other part is migrating scripts/console-library-generator.js to typescript. I guess you did this as part of checking how the code works, and it doesn't hurt to commit it.
The first part isolates the logic in ConsoleLogger - the class did not in fact depend on some initial state but rather just used the selectors that are static/const and have to be generated by the script.
After the refactor we know that we only need to depend on a single function to correctly format the logs in the EDR and it will be easier to port and use it directly rather than having to roundtrip via FFI only to format the console logs.
The script that was originally used to generate the data used by the ConsoleLogger was discovered to be broken in the meantime (there's no bufferToInt anymore), so I migrated it to TS to ensure we catch any such regressions in the future (even if a short one, as we want to port it anyways) and made sure it still works as expected.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fvictorio
changed the title
Part of NomicFoundation/edr#246
The starting point for this PR was analyzing how we create the Provider and how different pieces interact with each other in JS and Rust<>JS in general around the stack traces logic and tests.
This removes some of the dead or redundant code and refreshes the console.log signature lookup table generation script. If we decide to move it to Rust, we won't have to call JS anymore and move buffers around just to decode and execute the console.log formatting logic.
After moving the auxiliary script to TS I re-ran it to verify that it outputs the same code (incl. the changes from this PR).