fix(router): fragment must be a string#37334
fix(router): fragment must be a string#37334MatthiasKunnen wants to merge 2 commits intoangular:masterfrom
Conversation
This tests for angular#23894.
ActivatedRoute.fragment was typed as Observable<string> but could emit both null and undefined due to incorrect non-null assertion. These non-null assertions have been removed and fragment has been ensured to be a string. Fragment being an empty string is consistent with window.location.hash. BREAKING CHANGE: ActivatedRoute.fragment will no longer be null when the URL does not have a fragment. Difference for fragment values: | | Old | New | |--------------|------|-----| | https://e.g | null | '' | | https://e.g# | '' | '' | Fixes angular#23894.
MatthiasKunnen
force-pushed
the
issue-23894-fragment-null
branch
from
69863a6 to
f87fd49
Compare
|
Hi @MatthiasKunnen, as I mentioned in the issue report, I think the correct fix is to update the type rather than convert null to empty string. #23894 (comment) |
|
I was already making this change and missed your comment. Two questions about changing the type:
|
|
Might be interesting, according to the spec internally, the URL fragment is |
|
Undefined should be converted to null. You can also mark your or as fixing this: #34197 The answer to your second question I think is a bit harder to answer. I personally think that should be empty string( see #29683 (comment) ). What’s the current behavior? |
|
Current behavior is an empty string |
|
I propose I close this PR and make a new one since this PR's intentions have changed and it might get confusing. Agreed? |
Yea, that sounds good. Thanks! |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
3 participants
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
ActivatedRoute.fragmentis typed asObservable<string>but can emit bothnullandundefineddue to incorrect non-null assertion.Issue Number: Fixes #23894
What is the new behavior?
The non-null assertions have been removed and fragment will now be an empty string instead of null. This behavior is consistent with window.location.hash.
Does this PR introduce a breaking change?
ActivatedRoute.fragment will no longer be null when the URL does not have a fragment. Difference for fragment values:
null''''''Applications should check for an empty string instead of
nullin order to determine that no fragment is set.