Skip to content

Conversation

alan-agius4
Copy link
Collaborator

@alan-agius4 alan-agius4 commented Sep 26, 2024

fix(@angular/ssr): show error when multiple routes are set with RenderMode.AppShell**

This change introduces error handling to ensure that when multiple routes are configured with RenderMode.AppShell, an error message is displayed. This prevents misconfiguration and enhances clarity in route management.


fix(@angular/ssr): improve handling of route mismatches between Angular server routes and Angular router

This commit resolves an issue where routes defined in the Angular server routing configuration did not match those in the Angular router. Previously, discrepancies between these routes went unnoticed by users. With this update, appropriate error messages are now displayed when mismatches occur, enhancing the developer experience and facilitating easier troubleshooting.

@alan-agius4 alan-agius4 added action: review The PR is still awaiting reviews from at least one requested reviewer target: major This PR is targeted for the next major release labels Sep 26, 2024
@alan-agius4 alan-agius4 force-pushed the error-unknown-route branch 2 times, most recently from 1521377 to 7fd2fdc Compare September 26, 2024 09:54
…ar server routes and Angular router

This commit resolves an issue where routes defined in the Angular server routing configuration did not match those in the Angular router. Previously, discrepancies between these routes went unnoticed by users. With this update, appropriate error messages are now displayed when mismatches occur, enhancing the developer experience and facilitating easier troubleshooting.
@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label Sep 26, 2024
Copy link

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

LGTM (just left a few minor comments) 👍

route: currentRoutePath,
};

delete metadata.matched;

Choose a reason for hiding this comment

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

The delete operation used to cause deopt in VM, may be use null or undefined instead?

Suggested change
delete metadata.matched;
metadata.matched = null;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is done during build time so I am less worried about that, if we set undefined or null, we’d need to filter it out so not to include it in the manifest

@alan-agius4 alan-agius4 added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Sep 26, 2024
…erMode.AppShell`

This change introduces error handling to ensure that when multiple routes are configured with `RenderMode.AppShell`, an error message is displayed. This prevents misconfiguration and enhances clarity in route management.
@angular-robot angular-robot bot removed the detected: feature PR contains a feature commit label Sep 26, 2024
@alan-agius4 alan-agius4 merged commit 64c5252 into angular:main Sep 26, 2024
30 checks passed
@alan-agius4 alan-agius4 deleted the error-unknown-route branch September 26, 2024 20:50
@angular-automatic-lock-bot
Copy link

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 Oct 27, 2024
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: @angular/ssr target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants