Skip to content

refactor(core): Add custom formatters for Signals#64000

Closed
JeanMeche wants to merge 1 commit intoangular:mainfrom
JeanMeche:custom-formatters
Closed

refactor(core): Add custom formatters for Signals#64000
JeanMeche wants to merge 1 commit intoangular:mainfrom
JeanMeche:custom-formatters

Conversation

@JeanMeche
Copy link
Copy Markdown
Member

@JeanMeche JeanMeche commented Sep 22, 2025

This commit adds a devmode only formatter for Angular signals

Custom formatters must also be enabled in the browser devtools.

Demo:
Screenshot 2025-09-23 at 00 17 14

TESTED= This change doesn't require a TGP, it doesn't affect existing code

@JeanMeche JeanMeche requested a review from dgp1130 September 22, 2025 22:16
@angular-robot angular-robot bot added the area: core Issues related to the framework runtime label Sep 22, 2025
@ngbot ngbot bot added this to the Backlog milestone Sep 22, 2025
Copy link
Copy Markdown
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.

Some quick initial thoughts, I recognize enough of my own code to know I'm critiquing myself as much as you. 😅

@JeanMeche JeanMeche force-pushed the custom-formatters branch 3 times, most recently from 8eca1d8 to 06a3d3b Compare September 23, 2025 00:28
@JeanMeche JeanMeche marked this pull request as ready for review September 23, 2025 11:13
@pullapprove pullapprove bot added the requires: TGP This PR requires a passing TGP before merging is allowed label Sep 23, 2025
Copy link
Copy Markdown
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

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

LGTM for public API (even though this is technically excluded anyway, via https://github.com/angular/angular/blob/main/contributing-docs/public-api-surface.md)

Reviewed-for: public-api

Copy link
Copy Markdown
Contributor

@thePunderWoman thePunderWoman left a comment

Choose a reason for hiding this comment

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

It feels a bit weird to me to put a devtools related formatter inside the primitives folder. I would expect the primitives folder to contain solely things that are primitives related.

@JeanMeche
Copy link
Copy Markdown
Member Author

This is directly related to the primitives themselves, here the signals. I would expect both Angular and wiz to actually use that formatter.

@thePunderWoman
Copy link
Copy Markdown
Contributor

@JeanMeche But would the wiz team be importing this itself from the primitives folder or would they just be installing devtools? I'm just wondering if it's better to organize this with the devtools code rather than inside the signals codebase. Would this ever be used outside of devtools?

@JeanMeche
Copy link
Copy Markdown
Member Author

This is not related to the Angular devtools, but the browser devtools themselves. A custom formatter influences how references are displayed in the sources, the console, etc.

@thePunderWoman
Copy link
Copy Markdown
Contributor

@JeanMeche Thank you. That clarifies it for me. 👍🏻

Copy link
Copy Markdown
Contributor

@thePunderWoman thePunderWoman left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
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.

The Wiz point is a good callout, I can mention this to some Wiz folks once it lands to make sure we install the formatter automatically in the right place so Wiz gets the same benefit.

@JeanMeche JeanMeche force-pushed the custom-formatters branch 3 times, most recently from e56b7c6 to 4405983 Compare September 24, 2025 16:05
@JeanMeche JeanMeche added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Sep 24, 2025
@JeanMeche
Copy link
Copy Markdown
Member Author

Caretaker note: This change doesn't require a TGP, it doesn't affect existing code

@JeanMeche JeanMeche added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release labels Sep 25, 2025
@JeanMeche
Copy link
Copy Markdown
Member Author

JeanMeche commented Sep 25, 2025

Here is the to fix the patch CL/810918799

@JeanMeche JeanMeche removed the action: merge The PR is ready for merge by the caretaker label Sep 26, 2025
@mturco mturco self-requested a review September 26, 2025 15:07
@JeanMeche
Copy link
Copy Markdown
Member Author

Presubmit is failing and I'm not sure why.

Copy link
Copy Markdown
Contributor

@mturco mturco left a comment

Choose a reason for hiding this comment

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

This is pretty cool! And fwiw I agree that it being in the primitives code makes sense

Reviewed-for: primitives-shared

@thePunderWoman thePunderWoman removed the request for review from tbondwilkinson September 26, 2025 15:46
@dgp1130
Copy link
Copy Markdown
Contributor

dgp1130 commented Sep 26, 2025

@mturco, once this lands, we should figure out where the right place in Wiz is to install it. I feel like this should "just work" for Wiz developers as well.

@JeanMeche JeanMeche force-pushed the custom-formatters branch 3 times, most recently from 039306f to 9230be6 Compare September 29, 2025 22:51
This commit adds a devmode only formatter for Angular signals

Custom formatters must also be enabled in the browser devtools.
@JeanMeche JeanMeche added action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note and removed merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note labels Sep 30, 2025
@kirjs
Copy link
Copy Markdown
Contributor

kirjs commented Sep 30, 2025

TESTED=This change doesn't require a TGP, it doesn't affect existing code

@kirjs
Copy link
Copy Markdown
Contributor

kirjs commented Sep 30, 2025

This PR was merged into the repository. The changes were merged into the following branches:

kirjs pushed a commit that referenced this pull request Sep 30, 2025
This commit adds a devmode only formatter for Angular signals

Custom formatters must also be enabled in the browser devtools.

PR Close #64000
@kirjs kirjs closed this in 2aca6da Sep 30, 2025
@JeanMeche JeanMeche deleted the custom-formatters branch October 10, 2025 12:55
napulitanfrontend pushed a commit to napulitanfrontend/angular that referenced this pull request Oct 10, 2025
This commit adds a devmode only formatter for Angular signals

Custom formatters must also be enabled in the browser devtools.

PR Close angular#64000
@angular-automatic-lock-bot
Copy link
Copy Markdown

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-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Nov 10, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note requires: TGP This PR requires a passing TGP before merging is allowed target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants