Implement Symbol.for("debug.properties") for custom property replacement in debugger#2311
Implement Symbol.for("debug.properties") for custom property replacement in debugger#2311
Conversation
|
@copilot please show me js code I can use to test this when i debug it |
I've created comprehensive test files for you! Here are three files ready to use: 1. Node.js Test ( class Observable {
constructor(value) {
this._value = value;
this._observers = new Set(['observer1', 'observer2', 'observer3']);
this._scheduler = { name: 'scheduler', details: 'complex object' };
this._isDisposed = false;
}
[Symbol.for('debug.properties')]() {
return {
value: this._value,
subscriberCount: this._observers.size
};
}
[Symbol.for('debug.description')]() {
return `Observable(${this._value})`;
}
}
const observable = new Observable('test value');
debugger; // Inspect 'observable' hereQuick start: node --inspect-brk test-debug-properties.js
# Then attach VS Code debugger2. Browser Test ( 3. VS Code Launch Config ( Test objects included:
What to look for:
All files are in |
…placement in debugger
9ff9326 to
4330850
Compare
| /** | ||
| * Separate function that initializes the properties symbol for custom property replacement. | ||
| */ | ||
| export const getPropertiesSymbol = remoteFunction(function() { |
There was a problem hiding this comment.
@copilot don't use a separate function for this, you can do this in getDescriptionSymbols
There was a problem hiding this comment.
Done. Merged getPropertiesSymbol into getDescriptionSymbols which now returns an array of 3 symbols: [Generic, Node, Properties]. Updated getCustomProperties to access the properties symbol via runtimeArgs[0][2]. (c7ea916)
src/adapter/variableStore.ts
Outdated
| // Check for Symbol.for("debug.properties") custom property replacement | ||
| if (!this.settings.customPropertiesGenerator && !skipSymbolBasedCustomProperties) { | ||
| try { | ||
| const customPropsResult = await this.cdp.Runtime.callFunctionOn({ |
There was a problem hiding this comment.
We don't want to pay a round trip cost on every single object representation. In practice only a very small minority of objects will define custom properties.
Can we instead only trigger this path if we see the custom property symbol as part of whichever of accessorsProperties, ownProperties, stringyProps the symbol would appear as?
Implement the recursion/loop in a pragmatic way that avoids duplication
There was a problem hiding this comment.
Implemented the optimization. Now checking for the presence of Symbol.for("debug.properties") in the object's properties (by looking for p.symbol?.description === 'Symbol(debug.properties)') before making the round trip. Only calls getCustomProperties if the symbol is found. (c7ea916)
…ts without the symbol
- Merged getPropertiesSymbol into getDescriptionSymbols to return array with 3 symbols
- Check for debug.properties symbol presence before making round trip
- Only call getCustomProperties if Symbol.for("debug.properties") is found in object properties
- Updated getCustomProperties to use symbols array from runtimeArgs[0][2]
Co-authored-by: connor4312 <2230985+connor4312@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
|
||
| if (hasDebugPropertiesSymbol) { | ||
| try { | ||
| const customPropsResult = await this.cdp.Runtime.callFunctionOn({ |
There was a problem hiding this comment.
there's a lot of duplication here, you can do better @copilot
There was a problem hiding this comment.
Refactored to eliminate duplication. Extracted the property fetching logic into a fetchObjectProperties helper method that's now used in both the initial fetch and when re-fetching for custom properties. (3773fc2)
…ication Extracted the duplicated code for fetching object properties (accessors, own properties, and stringy props) into a reusable private method fetchObjectProperties(). This eliminates the duplication that existed when re-fetching properties for custom property objects. Co-authored-by: connor4312 <2230985+connor4312@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Implementation Plan for Symbol-based Property Replacement
getCustomPropertiesforSymbol.for("debug.properties")supportVariableContext.createObjectPropertyVarsto check for and apply custom propertiesSummary
Latest changes:
fetchObjectPropertieshelper method to eliminate duplicationOriginal prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.