feat(devtools): implement iframe support by AleksanderBodurri · Pull Request #53934 · angular/angular

AleksanderBodurri · 2024-01-16T04:58:06Z

Introduces Iframe support into Angular DevTools, see individual commits

dgp1130

Thanks for putting this together! Most of these comments are relatively minor.

I would love to see some automated tests here. There are a ton of if { return; } and null checks which comprehensive unit tests would help add a lot of confidence for. Is there anything we can do in that direction?

dgp1130

Thanks for working through all these comments! It's definitely looking a lot cleaner. Looking forward to the tests for this PR.

dgp1130 · 2024-01-19T23:00:24Z

Suggestion: I feel like we're repeating this logic in a few places. I wonder if we could move it into some kind of FrameManager.getSelectedFrame() function which would return the current frame or undefined if the top-level frame and warn / abort if multiple frames conflict? Basically a slightly more powerful version of the selectedFrame getter you already have.

It's a little awkward because undefined would be the top frame and also the case where there are multiple, so we might need a discriminated union there which adds complexity.

Related question: Do we need undefined to point at the top-level frame? Could we just use the frame ID in that case and simplify drop the undefined case in all this logic, or does that further complicate things?

Not sure I'm following exactly but I replaced this with a more natural feeling frameHasUniqueUrl(frame: Frame | null): boolean that handles the null case by returning true

I meant something like:

export class FrameManager { getSelectedFrame(): Frame | null { if (this._selectedFrameId === null) { return null; } const frame = this._frames.get(this._selectedFrameId); if (!frame) return null; if (!this.frameHasUniqueUrl(frame)) { this._messageBus.emit('log', [ /* ... */ ]); return null; } return frame; } }

This way getSelectedFrame automatically checks if the frame is unique so each usage doesn't need to do that.

I'm not sure if we ever need to distinguish between the _selectedFrameId === null case and the !this.frameAsUniqueUrl(frame) case, so hopefully return null; works in both of them without creating ambiguity. Otherwise we might need a discriminated union to disambiguate them.

AndrewKushnir · 2024-02-15T01:15:29Z

This PR was merged into the repository by commit ebcdc8d.

…3934) In the Angular DevTools Chrome DevTools page: - Angular DevTools is able to ask the background script to list each frame that has been registered on a page. - Angular Devtools is able to ask the background script to "enable" the connection on a particular frame. This enables the messaging between the content script <-> background script <-> devtools page - Implements detection of non unique urls on the inspected page Limitations: - The `inspectedWindow.eval` API is only able to target frames by frameURL. This means some features that integrate with Chrome DevTools like inspect element and open source will not be available when inspecting frames that do not have a unique url on the page. PR Close #53934

…owser code (#53934) Modifies the messaging layer of devtools to allow for switching communication between frames on a page. When served as a browser extension. Design: - When a page renders, DevTools installs a content script onto it through it's manifest file. The all_frames option is used here to install this script onto every frame in a page. - When Angular is detected, the content script will install a backend script into it's frame. - Each content script / backend script pairing is kept track of in the background script. This pairing represents an angular devtools context in a particular frame. - Angular DevTools is able to ask the background script to list each frame that has been registered on a page. - Angular Devtools is able to ask the background script to "enable" the connection on a particular frame. This enables the messaging between the content script <-> background script <-> devtools page Limitations: - The `inspectedWindow.eval` API is only able to target frames by frameURL. This means some features that integrate with Chrome DevTools like inspect element and open source will not be available when inspecting frames that do not have a unique url on the page. PR Close #53934

…3934) In the Angular DevTools Chrome DevTools page: - Angular DevTools is able to ask the background script to list each frame that has been registered on a page. - Angular Devtools is able to ask the background script to "enable" the connection on a particular frame. This enables the messaging between the content script <-> background script <-> devtools page - Implements detection of non unique urls on the inspected page Limitations: - The `inspectedWindow.eval` API is only able to target frames by frameURL. This means some features that integrate with Chrome DevTools like inspect element and open source will not be available when inspecting frames that do not have a unique url on the page. PR Close #53934

…owser code (angular#53934) Modifies the messaging layer of devtools to allow for switching communication between frames on a page. When served as a browser extension. Design: - When a page renders, DevTools installs a content script onto it through it's manifest file. The all_frames option is used here to install this script onto every frame in a page. - When Angular is detected, the content script will install a backend script into it's frame. - Each content script / backend script pairing is kept track of in the background script. This pairing represents an angular devtools context in a particular frame. - Angular DevTools is able to ask the background script to list each frame that has been registered on a page. - Angular Devtools is able to ask the background script to "enable" the connection on a particular frame. This enables the messaging between the content script <-> background script <-> devtools page Limitations: - The `inspectedWindow.eval` API is only able to target frames by frameURL. This means some features that integrate with Chrome DevTools like inspect element and open source will not be available when inspecting frames that do not have a unique url on the page. PR Close angular#53934

…gular#53934) In the Angular DevTools Chrome DevTools page: - Angular DevTools is able to ask the background script to list each frame that has been registered on a page. - Angular Devtools is able to ask the background script to "enable" the connection on a particular frame. This enables the messaging between the content script <-> background script <-> devtools page - Implements detection of non unique urls on the inspected page Limitations: - The `inspectedWindow.eval` API is only able to target frames by frameURL. This means some features that integrate with Chrome DevTools like inspect element and open source will not be available when inspecting frames that do not have a unique url on the page. PR Close angular#53934

…page (angular#53934)" This reverts commit ebcdc8d.

…ools' browser code (angular#53934)" This reverts commit dd3dac9.

…page (#53934)" (#54629) This reverts commit ebcdc8d. PR Close #54629

…ools' browser code (#53934)" (#54629) This reverts commit dd3dac9. PR Close #54629

…page (#53934)" (#54629) This reverts commit ebcdc8d. PR Close #54629

…ools' browser code (#53934)" (#54629) This reverts commit dd3dac9. PR Close #54629

…lar DevTools' browser code (angular#53934)" (angular#54629)" This reverts commit dd9f9d7.

…evtools page (angular#53934)" (angular#54629)" This reverts commit 133319e.

angular-automatic-lock-bot · 2024-03-17T00:09:32Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

angular-robot Bot added the detected: feature PR contains a feature commit label Jan 16, 2024

AleksanderBodurri requested a review from dgp1130 January 16, 2024 04:58