Skip to content

feat(devtools): add transfer state tab #62465

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(devtools): add transfer state tab #62465

wants to merge 1 commit into from

Conversation

Avcharov
Copy link
Contributor

@Avcharov Avcharov commented Jul 4, 2025 •
edited
Loading

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.dev application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

transfer state is visible only on elements tab in ng-state script.

Issue Number: #62237

What is the new behavior?

Did workaround with adding of new transfer state tab. Still consider solution with transfer state list in components tab intead of proposed. Let me know which one is prefered.

Demo:
Click here for Demo Video

Снимок экрана 2025-07-04 в 10 30 01

Does this PR introduce a breaking change?

  • Yes
  • No

@pullapprove pullapprove bot requested a review from AleksanderBodurri July 4, 2025 08:04
@angular-robot angular-robot bot added detected: feature PR contains a feature commit area: devtools labels Jul 4, 2025
@ngbot ngbot bot added this to the Backlog milestone Jul 4, 2025
@Avcharov
Copy link
Contributor Author

Avcharov commented Jul 4, 2025

Help needed:

  • In presented solution it looks for transferState script only by ng-state id. However if APP_ID would not be default it will not work. How to inject APP_ID from debugged applications?
  • I used material for layout if needed can be edited to custom solution.

alan-agius4
alan-agius4 reviewed Jul 4, 2025
View reviewed changes
@Avcharov Avcharov force-pushed the devtools/transfer-state-tab branch 2 times, most recently from eb9c225 to a331ee5 Compare July 7, 2025 09:44
@pullapprove pullapprove bot requested a review from AndrewKushnir July 7, 2025 09:44
@Avcharov Avcharov force-pushed the devtools/transfer-state-tab branch from a331ee5 to 5ca05aa Compare July 8, 2025 11:53
JeanMeche
JeanMeche reviewed Jul 8, 2025
View reviewed changes
packages/core/src/render3/util/transfer_state_utils.ts Outdated
Comment on lines 22 to 35
const doc = getDocument();

const appId = injector.get(APP_ID);
const scriptId = appId + '-state';
const script = doc.getElementById(scriptId) as HTMLScriptElement;

if (!script) {
return {};
}

if (!script.textContent || script.textContent.trim() === '') {
return {};
}
return JSON.parse(script.textContent) as Record<string, unknown>;
Copy link
Member

@JeanMeche JeanMeche Jul 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can basically be replaced by retrieveTransferredState() located in transfer_state.ts (we'll need to export it first).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect this function to strip internal framework keys from packages/core/src/hydration/utils.ts : __nghData__ & __nghDeferData__ which are used for (incremental) hydration and don't have real meaning for developers.

JeanMeche
JeanMeche reviewed Jul 8, 2025
View reviewed changes
devtools/projects/ng-devtools/src/lib/devtools-tabs/transfer-state/transfer-state.component.ts

@Component({
selector: 'ng-transfer-state',
standalone: true,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: those aren't necessary anymore (since v19).

Suggested change
standalone: true,

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can also remove it, thank you.

@Avcharov Avcharov force-pushed the devtools/transfer-state-tab branch from 5ca05aa to cf4c8bb Compare July 13, 2025 13:00
@Avcharov Avcharov requested review from JeanMeche and hawkgs July 13, 2025 18:43
JeanMeche
JeanMeche reviewed Jul 13, 2025
View reviewed changes
devtools/projects/ng-devtools/src/lib/devtools-tabs/transfer-state/transfer-state.component.ts Outdated
Comment on lines 155 to 159
const formatted = getFormattedValue(value);
if (isExpanded) {
return formatted;
}
return truncateText(formatted, maxLength);
Copy link
Member

@JeanMeche JeanMeche Jul 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should replace this with a pure CSS solution (https://developer.mozilla.org/en-US/docs/Web/CSS/line-clamp) ?
(which would be applied to the DOM node that invokes this)

dgp1130
dgp1130 approved these changes Jul 14, 2025
View reviewed changes
Copy link
Contributor

@dgp1130 dgp1130 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for putting this together, very cool feature!

No major concerns on my side, just a few minor suggestions and potential UX improvements.

devtools/projects/ng-devtools-backend/src/lib/client-event-subscribers.ts Outdated
return;
}

const transferStateData = ng.ɵgetTransferState?.(injector) as Record<string, TransferStateValue>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I think this should be | undefined since ng.ɵgetTransferState may not exist.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that currently if the protocol event data type and ɵgetTransferState return type diverge, we will get a compile-time error that they don't sufficiently overlap, but maybe it's safer to drop the assertion altogether?

@Avcharov Avcharov mentioned this pull request Jul 16, 2025
Open
2 tasks
@Avcharov Avcharov force-pushed the devtools/transfer-state-tab branch from cf4c8bb to 0581d10 Compare July 18, 2025 05:38
@Avcharov
Copy link
Contributor Author

upd: colors to match theme
Снимок экрана 2025-07-18 в 07 39 59

@Avcharov Avcharov requested review from dgp1130, JeanMeche and hawkgs July 18, 2025 08:26
@Avcharov Avcharov force-pushed the devtools/transfer-state-tab branch from 0581d10 to 93d523a Compare July 18, 2025 12:15
@Avcharov @JeanMeche
feat(devtools): add transfer state tab
9c20601 
Add transfer state tab, which is taking transfer state script by using APP_ID. Created internal api ɵgetTransferState to retrieve transfer state value from app into devtools app.
@JeanMeche JeanMeche force-pushed the devtools/transfer-state-tab branch from 93d523a to 9c20601 Compare July 18, 2025 23:53
@JeanMeche
Copy link
Member

@Avcharov I've allowed myself to force push an update following the merge of #62627

AndrewKushnir
AndrewKushnir reviewed Jul 19, 2025
View reviewed changes
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, just left one comment.

packages/core/src/render3/util/transfer_state_utils.ts
Comment on lines +31 to +32
delete transferState[TRANSFER_STATE_TOKEN_ID];
delete transferState[TRANSFER_STATE_DEFER_BLOCKS_INFO];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be great to add a test for this change, to make sure we do not "leak" other private fields (we may add more in the future) into the transfer state visualization.

Unrelated (for followup PRs): it'd be great to rename TRANSFER_STATE_TOKEN_ID -> TRANSFER_STATE_HYDRATION_INFO to better represent the role of the symbol.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @JeanMeche

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

@hawkgs hawkgs hawkgs left review comments

@alan-agius4 alan-agius4 alan-agius4 left review comments

@AndrewKushnir AndrewKushnir AndrewKushnir left review comments

@JeanMeche JeanMeche JeanMeche approved these changes

@dgp1130 dgp1130 Awaiting requested review from dgp1130

Assignees
No one assigned
Labels
area: devtools detected: feature PR contains a feature commit
Projects
None yet
Milestone
Backlog
Development

Successfully merging this pull request may close these issues.
6 participants
@Avcharov @JeanMeche @hawkgs @dgp1130 @alan-agius4 @AndrewKushnir