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.

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

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

Conversation

Avcharov
Copy link
Contributor

@Avcharov Avcharov commented Jul 4, 2025

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.

@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
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

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.


@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
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

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)

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.

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?

<ng-container matColumnDef="size">
<th mat-header-cell *matHeaderCellDef>Size</th>
<td mat-cell *matCellDef="let item">{{ item.size }}</td>
</ng-container>
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider: Should we put a tooltip on the column (maybe with a ? icon) indicating that this is uncompressed size?

I know on Angular CLI bundle budgets a lot of people get confused whether we're listing disk size or transfer size, so it might be helpful to be explicit.

Actually we do provide an estimated transfer size based on simple formula. @clydin do you remember how that works? Maybe it would be worth including an estimated transfer size here?

});
} catch (err) {
this.error.set(`Error loading transfer state: ${err}`);
this.isLoading.set(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider: Could we derive isLoading from whether either transferStateData and error have a value provided?

<th mat-header-cell *matHeaderCellDef>Value</th>
<td mat-cell *matCellDef="let item" class="value-cell">
<div class="value-container">
<pre
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider: Should this include <code>? Would that be better for accessibility?

...result
} = transferState;

return result;
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider: You can drop the _ destructures if you overwrite in a new object:

return {
  ...result,
  [TRANSFER_STATE_TOKEN_ID]: undefined,
  [TRANSFER_STATE_DEFER_BLOCKS_INFO]: undefined,
};

You might have to filter out keys with undefined values when consuming this. Alternatively, you can just delete the keys.

const transferState = retrieveTransferredState(doc, appId);
delete transferState[TRANSFER_STATE_TOKEN_ID];
delete transferState[TRANSFER_STATE_DEFER_BLOCKS_INFO];

@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
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.
@Avcharov Avcharov force-pushed the devtools/transfer-state-tab branch from 0581d10 to 93d523a Compare July 18, 2025 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: devtools detected: feature PR contains a feature commit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants