fix(router): fragment can be null (angular#37336)

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 retyped to
string | null.

BREAKING CHANGE:
Strict null checks will report on fragment potentially being null.
Migration path: add null check.

Fixes angular#23894, fixes angular#34197.

PR Close angular#37336

Author: MatthiasKunnen

goldens/public-api/router/router.d.ts

@@ -3,7 +3,7 @@ export declare class ActivatedRoute {
     component: Type<any> | string | null;
     data: Observable<Data>;
     get firstChild(): ActivatedRoute | null;
-    fragment: Observable<string>;
+    fragment: Observable<string | null>;
     outlet: string;
     get paramMap(): Observable<ParamMap>;
     params: Observable<Params>;
@@ -23,7 +23,7 @@ export declare class ActivatedRouteSnapshot {
     component: Type<any> | string | null;
     data: Data;
     get firstChild(): ActivatedRouteSnapshot | null;
-    fragment: string;
+    fragment: string | null;
     outlet: string;
     get paramMap(): ParamMap;
     params: Params;

packages/router/src/apply_redirects.ts

@@ -91,7 +91,7 @@ class ApplyRedirects {
         this.expandSegmentGroup(this.ngModule, this.config, rootSegmentGroup, PRIMARY_OUTLET);
     const urlTrees$ = expanded$.pipe(map((rootSegmentGroup: UrlSegmentGroup) => {
       return this.createUrlTree(
-          squashSegmentGroup(rootSegmentGroup), this.urlTree.queryParams, this.urlTree.fragment!);
+          squashSegmentGroup(rootSegmentGroup), this.urlTree.queryParams, this.urlTree.fragment);
     }));
     return urlTrees$.pipe(catchError((e: any) => {
       if (e instanceof AbsoluteRedirect) {
@@ -114,7 +114,7 @@ class ApplyRedirects {
         this.expandSegmentGroup(this.ngModule, this.config, tree.root, PRIMARY_OUTLET);
     const mapped$ = expanded$.pipe(map((rootSegmentGroup: UrlSegmentGroup) => {
       return this.createUrlTree(
-          squashSegmentGroup(rootSegmentGroup), tree.queryParams, tree.fragment!);
+          squashSegmentGroup(rootSegmentGroup), tree.queryParams, tree.fragment);
     }));
     return mapped$.pipe(catchError((e: any): Observable<UrlTree> => {
       if (e instanceof NoMatch) {
@@ -129,7 +129,7 @@ class ApplyRedirects {
     return new Error(`Cannot match any routes. URL Segment: '${e.segmentGroup}'`);
   }
 
-  private createUrlTree(rootCandidate: UrlSegmentGroup, queryParams: Params, fragment: string):
+  private createUrlTree(rootCandidate: UrlSegmentGroup, queryParams: Params, fragment: string|null):
       UrlTree {
     const root = rootCandidate.segments.length > 0 ?
         new UrlSegmentGroup([], {[PRIMARY_OUTLET]: rootCandidate}) :

packages/router/src/create_url_tree.ts

@@ -12,8 +12,8 @@ import {UrlSegment, UrlSegmentGroup, UrlTree} from './url_tree';
 import {forEach, last, shallowEqual} from './utils/collection';
 
 export function createUrlTree(
-    route: ActivatedRoute, urlTree: UrlTree, commands: any[], queryParams: Params,
-    fragment: string): UrlTree {
+    route: ActivatedRoute, urlTree: UrlTree, commands: any[], queryParams: Params|null,
+    fragment: string|null): UrlTree {
   if (commands.length === 0) {
     return tree(urlTree.root, urlTree.root, urlTree, queryParams, fragment);
   }
@@ -47,7 +47,7 @@ function isCommandWithOutlets(command: any): command is {outlets: {[key: string]
 
 function tree(
     oldSegmentGroup: UrlSegmentGroup, newSegmentGroup: UrlSegmentGroup, urlTree: UrlTree,
-    queryParams: Params, fragment: string): UrlTree {
+    queryParams: Params|null, fragment: string|null): UrlTree {
   let qp: any = {};
   if (queryParams) {
     forEach(queryParams, (value: any, name: any) => {

packages/router/src/recognize.ts

@@ -67,7 +67,7 @@ export class Recognizer {
     // Use Object.freeze to prevent readers of the Router state from modifying it outside of a
     // navigation, resulting in the router being out of sync with the browser.
     const root = new ActivatedRouteSnapshot(
-        [], Object.freeze({}), Object.freeze({...this.urlTree.queryParams}), this.urlTree.fragment!,
+        [], Object.freeze({}), Object.freeze({...this.urlTree.queryParams}), this.urlTree.fragment,
         {}, PRIMARY_OUTLET, this.rootComponentType, null, this.urlTree.root, -1, {});
 
     const rootNode = new TreeNode<ActivatedRouteSnapshot>(root, children);
@@ -160,7 +160,7 @@ export class Recognizer {
     if (route.path === '**') {
       const params = segments.length > 0 ? last(segments)!.parameters : {};
       snapshot = new ActivatedRouteSnapshot(
-          segments, params, Object.freeze({...this.urlTree.queryParams}), this.urlTree.fragment!,
+          segments, params, Object.freeze({...this.urlTree.queryParams}), this.urlTree.fragment,
           getData(route), getOutlet(route), route.component!, route,
           getSourceSegmentGroup(rawSegment), getPathIndexShift(rawSegment) + segments.length,
           getResolve(route));
@@ -174,7 +174,7 @@ export class Recognizer {
 
       snapshot = new ActivatedRouteSnapshot(
           consumedSegments, result.parameters, Object.freeze({...this.urlTree.queryParams}),
-          this.urlTree.fragment!, getData(route), getOutlet(route), route.component!, route,
+          this.urlTree.fragment, getData(route), getOutlet(route), route.component!, route,
           getSourceSegmentGroup(rawSegment),
           getPathIndexShift(rawSegment) + consumedSegments.length, getResolve(route));
     }

packages/router/src/router.ts

@@ -1142,7 +1142,7 @@ export class Router {
     if (q !== null) {
       q = this.removeEmptyProps(q);
     }
-    return createUrlTree(a, this.currentUrlTree, commands, q!, f!);
+    return createUrlTree(a, this.currentUrlTree, commands, q, f ?? null);
   }
 
   /**

packages/router/src/router_state.ts

@@ -126,7 +126,7 @@ export class ActivatedRoute {
       /** An observable of the query parameters shared by all the routes. */
       public queryParams: Observable<Params>,
       /** An observable of the URL fragment shared by all the routes. */
-      public fragment: Observable<string>,
+      public fragment: Observable<string|null>,
       /** An observable of the static and resolved data of this route. */
       public data: Observable<Data>,
       /** The outlet name of the route, a constant. */
@@ -321,7 +321,7 @@ export class ActivatedRouteSnapshot {
       /** The query parameters shared by all the routes */
       public queryParams: Params,
       /** The URL fragment shared by all the routes */
-      public fragment: string,
+      public fragment: string|null,
       /** The static and resolved data of this route */
       public data: Data,
       /** The outlet name of the route */

packages/router/src/url_tree.ts

@@ -382,7 +382,7 @@ export class DefaultUrlSerializer implements UrlSerializer {
     const segment = `/${serializeSegment(tree.root, true)}`;
     const query = serializeQueryParams(tree.queryParams);
     const fragment =
-        typeof tree.fragment === `string` ? `#${encodeUriFragment(tree.fragment!)}` : '';
+        typeof tree.fragment === `string` ? `#${encodeUriFragment(tree.fragment)}` : '';
 
     return `${segment}${query}${fragment}`;
   }

packages/router/test/create_url_tree.spec.ts

@@ -406,7 +406,7 @@ function createRoot(tree: UrlTree, commands: any[], queryParams?: Params, fragme
       new BehaviorSubject(null!), new BehaviorSubject(null!), new BehaviorSubject(null!),
       new BehaviorSubject(null!), new BehaviorSubject(null!), PRIMARY_OUTLET, 'someComponent', s);
   advanceActivatedRoute(a);
-  return createUrlTree(a, tree, commands, queryParams!, fragment!);
+  return createUrlTree(a, tree, commands, queryParams ?? null, fragment ?? null);
 }
 
 function create(
@@ -422,5 +422,5 @@ function create(
       new BehaviorSubject(null!), new BehaviorSubject(null!), new BehaviorSubject(null!),
       new BehaviorSubject(null!), new BehaviorSubject(null!), PRIMARY_OUTLET, 'someComponent', s);
   advanceActivatedRoute(a);
-  return createUrlTree(a, tree, commands, queryParams!, fragment!);
+  return createUrlTree(a, tree, commands, queryParams ?? null, fragment ?? null);
 }

packages/router/test/integration.spec.ts

@@ -1273,6 +1273,20 @@ describe('Integration', () => {
        expect(fixture.nativeElement).toHaveText('query: 2 fragment: fragment2');
      })));
 
+  it('should handle empty or missing fragments', fakeAsync(inject([Router], (router: Router) => {
+       const fixture = createRoot(router, RootCmp);
+
+       router.resetConfig([{path: 'query', component: QueryParamsAndFragmentCmp}]);
+
+       router.navigateByUrl('/query#');
+       advance(fixture);
+       expect(fixture.nativeElement).toHaveText('query:  fragment: ');
+
+       router.navigateByUrl('/query');
+       advance(fixture);
+       expect(fixture.nativeElement).toHaveText('query:  fragment: null');
+     })));
+
   it('should ignore null and undefined query params',
      fakeAsync(inject([Router], (router: Router) => {
        const fixture = createRoot(router, RootCmp);
@@ -6058,7 +6072,15 @@ class QueryParamsAndFragmentCmp {
 
   constructor(route: ActivatedRoute) {
     this.name = route.queryParamMap.pipe(map((p: ParamMap) => p.get('name')));
-    this.fragment = route.fragment;
+    this.fragment = route.fragment.pipe(map((p: string|null|undefined) => {
+      if (p === undefined) {
+        return 'undefined';
+      } else if (p === null) {
+        return 'null';
+      } else {
+        return p;
+      }
+    }));
   }
 }
 

packages/router/test/url_serializer.spec.ts

@@ -186,6 +186,12 @@ describe('url serializer', () => {
     expect(url.serialize(tree)).toEqual('/one#');
   });
 
+  it('should parse no fragment', () => {
+    const tree = url.parse('/one');
+    expect(tree.fragment).toEqual(null);
+    expect(url.serialize(tree)).toEqual('/one');
+  });
+
   describe('encoding/decoding', () => {
     it('should encode/decode path segments and parameters', () => {
       const u = `/${encodeUriSegment('one two')};${encodeUriSegment('p 1')}=${