From bc0fc020161f930a3b1ae5622fcc7aacaebf906a Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Thu, 27 Oct 2022 11:38:46 -0700 Subject: [PATCH] refactor(router): Warn when provideRoutes is used without provideRouter (#47896) Due to being only 1 letter away from `provideRouter`, it is quite possible that developers may accidentally use `provideRoutes` rather than `provideRouter` in the `boostrapApplication` function. This change will warn developers when `provideRoutes` is used without the `Router`. PR Close #47896 --- aio/content/guide/deprecations.md | 2 +- .../router/bundle.golden_symbols.json | 6 ++--- packages/router/src/provide_router.ts | 24 +++++++++++++++++++ packages/router/src/router_module.ts | 5 +++- packages/router/test/standalone.spec.ts | 13 ++++++++-- 5 files changed, 43 insertions(+), 7 deletions(-) diff --git a/aio/content/guide/deprecations.md b/aio/content/guide/deprecations.md index 4282abc6280..e4fbb9b6ee7 100644 --- a/aio/content/guide/deprecations.md +++ b/aio/content/guide/deprecations.md @@ -168,7 +168,7 @@ In the [API reference section](api) of this site, deprecated APIs are indicated | [`resolver` argument in `RouterOutletContract.activateWith`](api/router/RouterOutletContract#activatewith) | No replacement needed | v14 | Component factories are not required to create an instance of a component dynamically. Passing a factory resolver via `resolver` argument is no longer needed. | | [`resolver` field of the `OutletContext` class](api/router/OutletContext#resolver) | No replacement needed | v14 | Component factories are not required to create an instance of a component dynamically. Passing a factory resolver via `resolver` class field is no longer needed. | | [`RouterLinkWithHref` directive](api/router/RouterLinkWithHref) | Use `RouterLink` instead. | v15 | The `RouterLinkWithHref` directive code was merged into `RouterLink`. Now the `RouterLink` directive can be used for all elements that have `routerLink` attribute. | -| [`provideRoutes` function](api/router/provideRoutes) | Use `ROUTES` `InjectionToken` instead. | v15 | The `provideRoutes` helper function is minimally useful and can be unintentionally used instead of `provideRoutes` due similar spelling. | +| [`provideRoutes` function](api/router/provideRoutes) | Use `ROUTES` `InjectionToken` instead. | v15 | The `provideRoutes` helper function is minimally useful and can be unintentionally used instead of `provideRouter` due to similar spelling. | diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json index 616bd0f6552..5cea13d6743 100644 --- a/packages/core/test/bundling/router/bundle.golden_symbols.json +++ b/packages/core/test/bundling/router/bundle.golden_symbols.json @@ -1577,9 +1577,6 @@ { "name": "promise" }, - { - "name": "provideRoutes" - }, { "name": "redirectIfUrlTree" }, @@ -1640,6 +1637,9 @@ { "name": "rootRoute" }, + { + "name": "routes" + }, { "name": "rxSubscriber" }, diff --git a/packages/router/src/provide_router.ts b/packages/router/src/provide_router.ts index 6432082c7bf..2c96a85d13f 100644 --- a/packages/router/src/provide_router.ts +++ b/packages/router/src/provide_router.ts @@ -61,6 +61,7 @@ const NG_DEV_MODE = typeof ngDevMode === 'undefined' || ngDevMode; export function provideRouter(routes: Routes, ...features: RouterFeatures[]): EnvironmentProviders { return makeEnvironmentProviders([ {provide: ROUTES, multi: true, useValue: routes}, + NG_DEV_MODE ? {provide: ROUTER_IS_PROVIDED, useValue: true} : [], {provide: ActivatedRoute, useFactory: rootRoute, deps: [Router]}, {provide: APP_BOOTSTRAP_LISTENER, multi: true, useFactory: getBootstrapListener}, features.map(feature => feature.ɵproviders), @@ -93,6 +94,28 @@ function routerFeature( return {ɵkind: kind, ɵproviders: providers}; } + +/** + * An Injection token used to indicate whether `provideRouter` or `RouterModule.forRoot` was ever + * called. + */ +export const ROUTER_IS_PROVIDED = + new InjectionToken('', {providedIn: 'root', factory: () => false}); + +const routerIsProvidedDevModeCheck = { + provide: ENVIRONMENT_INITIALIZER, + multi: true, + useFactory() { + return () => { + if (!inject(ROUTER_IS_PROVIDED)) { + console.warn( + '`provideRoutes` was called without `provideRouter` or `RouterModule.forRoot`. ' + + 'This is likely a mistake.'); + } + }; + } +}; + /** * Registers a [DI provider](guide/glossary#provider) for a set of routes. * @param routes The route configuration to provide. @@ -113,6 +136,7 @@ function routerFeature( export function provideRoutes(routes: Routes): Provider[] { return [ {provide: ROUTES, multi: true, useValue: routes}, + NG_DEV_MODE ? routerIsProvidedDevModeCheck : [], ]; } diff --git a/packages/router/src/router_module.ts b/packages/router/src/router_module.ts index 0a8be6ac920..bb023c7f551 100644 --- a/packages/router/src/router_module.ts +++ b/packages/router/src/router_module.ts @@ -15,7 +15,7 @@ import {RouterLinkActive} from './directives/router_link_active'; import {RouterOutlet} from './directives/router_outlet'; import {RuntimeErrorCode} from './errors'; import {Routes} from './models'; -import {getBootstrapListener, rootRoute, withDebugTracing, withDisabledInitialNavigation, withEnabledBlockingInitialNavigation, withPreloading} from './provide_router'; +import {getBootstrapListener, rootRoute, ROUTER_IS_PROVIDED, withDebugTracing, withDisabledInitialNavigation, withEnabledBlockingInitialNavigation, withPreloading} from './provide_router'; import {Router, setupRouter} from './router'; import {ExtraOptions, ROUTER_CONFIGURATION} from './router_config'; import {RouterConfigLoader, ROUTES} from './router_config_loader'; @@ -48,6 +48,9 @@ export const ROUTER_PROVIDERS: Provider[] = [ ChildrenOutletContexts, {provide: ActivatedRoute, useFactory: rootRoute, deps: [Router]}, RouterConfigLoader, + // Only used to warn when `provideRoutes` is used without `RouterModule` or `provideRouter`. Can + // be removed when `provideRoutes` is removed. + NG_DEV_MODE ? {provide: ROUTER_IS_PROVIDED, useValue: true} : [], ]; export function routerNgProbeToken() { diff --git a/packages/router/test/standalone.spec.ts b/packages/router/test/standalone.spec.ts index e39dc9136a0..3f1a959c92b 100644 --- a/packages/router/test/standalone.spec.ts +++ b/packages/router/test/standalone.spec.ts @@ -9,9 +9,8 @@ import {Component, Injectable, NgModule} from '@angular/core'; import {ComponentFixture, fakeAsync, TestBed, tick} from '@angular/core/testing'; import {By} from '@angular/platform-browser'; -import {NavigationEnd, Router, RouterModule} from '@angular/router'; +import {provideRoutes, Router, RouterModule, ROUTES} from '@angular/router'; import {RouterTestingModule} from '@angular/router/testing'; -import {filter, first} from 'rxjs/operators'; @Component({template: '
simple standalone
', standalone: true}) export class SimpleStandaloneComponent { @@ -372,6 +371,16 @@ describe('standalone in Router API', () => { }); }); +describe('provideRoutes', () => { + it('warns if provideRoutes is used without provideRouter, RouterTestingModule, or RouterModule.forRoot', + () => { + spyOn(console, 'warn'); + TestBed.configureTestingModule({providers: [provideRoutes([])]}); + TestBed.inject(ROUTES); + expect(console.warn).toHaveBeenCalled(); + }); +}); + function advance(fixture: ComponentFixture) { tick(); fixture.detectChanges();