Skip to content

[XDebug Bridge] Fetch all array keys when inspecting an array - #2409

Merged
adamziel merged 24 commits into
trunkfrom
xdebug-paginate-props
Aug 25, 2025
Merged

adamziel merged 24 commits into
trunkfrom
xdebug-paginate-props

Conversation

@adamziel

@adamziel adamziel commented Jul 23, 2025 •

Copy link
Copy Markdown
Collaborator

Motivation for the change, related issues

Fixes two problems:

  • Fetches all the array keys when inspecting arrays. property_get XDebug command is paginated and before this PR, we'd only see a subset of all the array keys. With this PR, we see all the keys. This will be a problem on huge arrays so we'll need to bucket keys into group of 1000 in a follow-up PR.
  • Supports actually browsing superglobals, e.g. $_SERVER. Before this PR, you'd only see another nested superglobal (see the screenshot below).

As for the latter, I'm not too happy about the solution – we're reaching for the nested XDebug .property.property when it's available. I expect that will lead us astray in some scenarios, e.g. when browsing arrays that have nested arrays. That being said, I couldn't easily break it so let's get it in and continue iterating:

const responseProps =
	response.property?.property ?? response.property;

Testing instructions

Create a file called xdebug.php:

<?php
unset($_SERVER['CLIENT_SECRET'], $_SERVER['CLIENT_ID']);
$test = 42;  // Set a breakpoint on this line

$another = [15, [1,2,3, [4,5,6] ] ];

echo "Output!\n";

function test() {
	echo "Hello Xdebug World!\n";
print_R($_ENV);
print_r($_SERVER);
}

test();

Run nx run php-wasm-cli:dev --xdebug --experimental-devtools xdebug.php before and after this PR. Confirm you see the following results:

Before

CleanShot 2025-07-24 at 00 33 54@2x

After ($_SERVER is still duplicated – let's fix that separately)

CleanShot 2025-07-24 at 00 32 50@2x

Base automatically changed from add-cdp-option-in-cli to trunk July 24, 2025 08:10
@mho22 mho22 mentioned this pull request Jul 24, 2025
5 tasks
@mho22

mho22 commented Jul 28, 2025 •

Copy link
Copy Markdown
Collaborator

I think I found a way to avoid having that duplicate array inside itself. When paginating, the last element is the element itself with :

{
  '$': {
    name: '$_ENV',
    fullname: '$_ENV',
    type: 'array',
    children: '1',
    numchildren: '86',
    page: '3',
    pagesize: '32'
  }
}

So instead of injecting the object itself again in the array, let's check if the current page is actually '0' and add it. Do nothing otherwise.

xdebug-cdp-bridge.ts on line 795

	// Use same depth/context as parent
	this.objectHandles.set(childObjectId, {
		type: 'property',
		depth: depth,
		contextId: contextId,
		fullname: prop.$.fullname || name,
	});

+	if (prop.$.page == 0) {
		currentProps.push({
			name: prop.$.key || name,
			value: {
				type: 'object',
				className: className,
				description: className,
				objectId: childObjectId,
			},
			writable: false,
			configurable: false,
			enumerable: true,
		});
+	}
} else { 

@mho22

mho22 commented Aug 4, 2025

Copy link
Copy Markdown
Collaborator

@adamziel would you like me to take over this pull request?

@adamziel

adamziel commented Aug 4, 2025 •

Copy link
Copy Markdown
Collaborator Author

@mho22 yes please, thank you!

@mho22
mho22 requested a review from a team as a code owner August 4, 2025 14:56
@mho22

mho22 commented Aug 4, 2025 •

Copy link
Copy Markdown
Collaborator

I think your solution is great :

const responseProps =
	response.property?.property ?? response.property;

Based on my findings in a previous comment, it seems like the last element of an array is the array itself without its property property. I will try to find a nested array example to be sure it works as expected.

I will also try to add multiple tests to cover what has been added in this pull request.

@mho22
mho22 marked this pull request as draft August 4, 2025 15:09
@mho22

mho22 commented Aug 15, 2025 •

Copy link
Copy Markdown
Collaborator

I added the complete set of tests for the Xdebug bridge. And a specific test for the nested array relative to this pull request, the last test in the xdebug-cdp-bridge.spec.ts file. This pull request is now ready for review.

@adamziel
adamziel merged commit 8783e1e into trunk Aug 25, 2025
@adamziel
adamziel deleted the xdebug-paginate-props branch August 25, 2025 15:39
@adamziel

Copy link
Copy Markdown
Collaborator Author

Thank you @mho22!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants