Skip to content

Commit

Permalink
refactor(core): Throw a runtime error if both zone and zoneless are p…
Browse files Browse the repository at this point in the history
…rovided

This commit adds a dev-mode error if both the zone and zoneless
providers are used together.
  • Loading branch information
atscott committed May 7, 2024
1 parent 67bb310 commit 1d7bc27
Show file tree
Hide file tree
Showing 21 changed files with 120 additions and 35 deletions.
2 changes: 2 additions & 0 deletions goldens/public-api/core/errors.md
Expand Up @@ -117,6 +117,8 @@ export const enum RuntimeErrorCode {
// (undocumented)
PLATFORM_NOT_FOUND = 401,
// (undocumented)
PROVIDED_BOTH_ZONE_AND_ZONELESS = 408,
// (undocumented)
PROVIDER_IN_WRONG_CONTEXT = 207,
// (undocumented)
PROVIDER_NOT_FOUND = -201,
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/application/create_application.ts
Expand Up @@ -8,7 +8,7 @@

import {Subscription} from 'rxjs';

import {provideZoneChangeDetection} from '../change_detection/scheduling/ng_zone_scheduling';
import {internalProvideZoneChangeDetection} from '../change_detection/scheduling/ng_zone_scheduling';
import {EnvironmentProviders, Provider, StaticProvider} from '../di/interface/provider';
import {EnvironmentInjector} from '../di/r3_injector';
import {ErrorHandler} from '../error_handler';
Expand Down Expand Up @@ -55,7 +55,7 @@ export function internalCreateApplication(config: {

// Create root application injector based on a set of providers configured at the platform
// bootstrap level as well as providers passed to the bootstrap call by a user.
const allAppProviders = [provideZoneChangeDetection(), ...(appProviders || [])];
const allAppProviders = [internalProvideZoneChangeDetection({}), ...(appProviders || [])];
const adapter = new EnvironmentNgModuleRefAdapter({
providers: allAppProviders,
parent: platformInjector as EnvironmentInjector,
Expand Down
Expand Up @@ -26,7 +26,11 @@ import {NgZone} from '../../zone';
import {InternalNgZoneOptions} from '../../zone/ng_zone';

import {alwaysProvideZonelessScheduler} from './flags';
import {ChangeDetectionScheduler, ZONELESS_SCHEDULER_DISABLED} from './zoneless_scheduling';
import {
ChangeDetectionScheduler,
ZONELESS_ENABLED,
ZONELESS_SCHEDULER_DISABLED,
} from './zoneless_scheduling';
import {ChangeDetectionSchedulerImpl} from './zoneless_scheduling_impl';

@Injectable({providedIn: 'root'})
Expand Down Expand Up @@ -74,9 +78,10 @@ export function internalProvideZoneChangeDetection({
ngZoneFactory,
ignoreChangesOutsideZone,
}: {
ngZoneFactory: () => NgZone;
ngZoneFactory?: () => NgZone;
ignoreChangesOutsideZone?: boolean;
}): StaticProvider[] {
ngZoneFactory ??= () => new NgZone(getNgZoneOptions());
return [
{provide: NgZone, useFactory: ngZoneFactory},
{
Expand Down Expand Up @@ -161,7 +166,7 @@ export function provideZoneChangeDetection(options?: NgZoneOptions): Environment
});
return makeEnvironmentProviders([
typeof ngDevMode === 'undefined' || ngDevMode
? {provide: PROVIDED_NG_ZONE, useValue: true}
? [{provide: PROVIDED_NG_ZONE, useValue: true}, bothZoneAndZonelessErrorCheckProvider]
: [],
zoneProviders,
]);
Expand Down Expand Up @@ -295,3 +300,19 @@ export class ZoneStablePendingTask {
this.subscription.unsubscribe();
}
}

const bothZoneAndZonelessErrorCheckProvider = {
provide: ENVIRONMENT_INITIALIZER,
multi: true,
useFactory: () => {
const providedZoneless = inject(ZONELESS_ENABLED, {optional: true});
if (providedZoneless) {
throw new RuntimeError(
RuntimeErrorCode.PROVIDED_BOTH_ZONE_AND_ZONELESS,
'Invalid change detection configuration: ' +
'provideZoneChangeDetection and provideExperimentalZonelessChangeDetection cannot be used together.',
);
}
return () => {};
},
};
2 changes: 1 addition & 1 deletion packages/core/src/core_private_export.ts
Expand Up @@ -21,12 +21,12 @@ export {
defaultIterableDiffers as ɵdefaultIterableDiffers,
defaultKeyValueDiffers as ɵdefaultKeyValueDiffers,
} from './change_detection/change_detection';
export {internalProvideZoneChangeDetection as ɵinternalProvideZoneChangeDetection} from './change_detection/scheduling/ng_zone_scheduling';
export {
ChangeDetectionScheduler as ɵChangeDetectionScheduler,
NotificationSource as ɵNotificationSource,
ZONELESS_ENABLED as ɵZONELESS_ENABLED,
} from './change_detection/scheduling/zoneless_scheduling';
export {PROVIDED_NG_ZONE as ɵPROVIDED_NG_ZONE} from './change_detection/scheduling/ng_zone_scheduling';
export {Console as ɵConsole} from './console';
export {
DeferBlockDetails as ɵDeferBlockDetails,
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/errors.ts
Expand Up @@ -69,6 +69,7 @@ export const enum RuntimeErrorCode {
ASYNC_INITIALIZERS_STILL_RUNNING = 405,
APPLICATION_REF_ALREADY_DESTROYED = 406,
RENDERER_NOT_FOUND = 407,
PROVIDED_BOTH_ZONE_AND_ZONELESS = 408,

// Hydration Errors
HYDRATION_NODE_MISMATCH = -500,
Expand Down
12 changes: 12 additions & 0 deletions packages/core/src/platform/platform_ref.ts
Expand Up @@ -20,6 +20,7 @@ import {
internalProvideZoneChangeDetection,
PROVIDED_NG_ZONE,
} from '../change_detection/scheduling/ng_zone_scheduling';
import {ZONELESS_ENABLED} from '../change_detection/scheduling/zoneless_scheduling';
import {Injectable, InjectionToken, Injector} from '../di';
import {ErrorHandler} from '../error_handler';
import {RuntimeError, RuntimeErrorCode} from '../errors';
Expand Down Expand Up @@ -103,6 +104,17 @@ export class PlatformRef {
'`bootstrapModule` does not support `provideZoneChangeDetection`. Use `BootstrapOptions` instead.',
);
}
if (
(typeof ngDevMode === 'undefined' || ngDevMode) &&
moduleRef.injector.get(ZONELESS_ENABLED, null) &&
options?.ngZone !== 'noop'
) {
throw new RuntimeError(
RuntimeErrorCode.PROVIDED_BOTH_ZONE_AND_ZONELESS,
'Invalid change detection configuration: ' +
"`ngZone: 'noop'` must be set in `BootstrapOptions` with provideExperimentalZonelessChangeDetection.",
);
}

const exceptionHandler = moduleRef.injector.get(ErrorHandler, null);
if ((typeof ngDevMode === 'undefined' || ngDevMode) && exceptionHandler === null) {
Expand Down
33 changes: 33 additions & 0 deletions packages/core/test/acceptance/bootstrap_spec.ts
Expand Up @@ -14,6 +14,7 @@ import {
forwardRef,
NgModule,
NgZone,
provideExperimentalZonelessChangeDetection,
TestabilityRegistry,
ViewContainerRef,
ViewEncapsulation,
Expand Down Expand Up @@ -327,6 +328,38 @@ describe('bootstrap', () => {
}),
);

it(
'should throw when using zoneless without ngZone: "noop"',
withBody('<my-app></my-app>', async () => {
@Component({
template: '...',
})
class App {}

@NgModule({
declarations: [App],
providers: [provideExperimentalZonelessChangeDetection()],
imports: [BrowserModule],
bootstrap: [App],
})
class MyModule {}

try {
await platformBrowserDynamic().bootstrapModule(MyModule);

// This test tries to bootstrap a standalone component using NgModule-based bootstrap
// mechanisms. We expect standalone components to be bootstrapped via
// `bootstrapApplication` API instead.
fail('Expected to throw');
} catch (e: unknown) {
const expectedErrorMessage =
"Invalid change detection configuration: `ngZone: 'noop'` must be set in `BootstrapOptions`";
expect(e).toBeInstanceOf(Error);
expect((e as Error).message).toContain(expectedErrorMessage);
}
}),
);

it(
'should throw when standalone component wrapped in `forwardRef` is used in @NgModule.bootstrap',
withBody('<my-app></my-app>', async () => {
Expand Down
Expand Up @@ -1082,6 +1082,9 @@
{
"name": "internalImportProvidersFrom"
},
{
"name": "internalProvideZoneChangeDetection"
},
{
"name": "interpolateParams"
},
Expand Down Expand Up @@ -1292,9 +1295,6 @@
{
"name": "profiler"
},
{
"name": "provideZoneChangeDetection"
},
{
"name": "refreshContentQueries"
},
Expand Down
Expand Up @@ -1025,6 +1025,9 @@
{
"name": "getNextLContainer"
},
{
"name": "getNgZoneOptions"
},
{
"name": "getNodeInjectable"
},
Expand Down
Expand Up @@ -797,6 +797,9 @@
{
"name": "getNextLContainer"
},
{
"name": "getNgZoneOptions"
},
{
"name": "getNodeInjectable"
},
Expand Down
6 changes: 3 additions & 3 deletions packages/core/test/bundling/defer/bundle.golden_symbols.json
Expand Up @@ -2114,6 +2114,9 @@
{
"name": "internalImportProvidersFrom"
},
{
"name": "internalProvideZoneChangeDetection"
},
{
"name": "invokeAllTriggerCleanupFns"
},
Expand Down Expand Up @@ -2309,9 +2312,6 @@
{
"name": "profiler"
},
{
"name": "provideZoneChangeDetection"
},
{
"name": "refreshContentQueries"
},
Expand Down
Expand Up @@ -1112,6 +1112,9 @@
{
"name": "getNgDirectiveDef"
},
{
"name": "getNgZoneOptions"
},
{
"name": "getNodeInjectable"
},
Expand Down
Expand Up @@ -1079,6 +1079,9 @@
{
"name": "getNgDirectiveDef"
},
{
"name": "getNgZoneOptions"
},
{
"name": "getNodeInjectable"
},
Expand Down
Expand Up @@ -632,6 +632,9 @@
{
"name": "getNextLContainer"
},
{
"name": "getNgZoneOptions"
},
{
"name": "getNodeInjectable"
},
Expand Down
Expand Up @@ -998,6 +998,9 @@
{
"name": "internalImportProvidersFrom"
},
{
"name": "internalProvideZoneChangeDetection"
},
{
"name": "invokeHostBindingsInCreationMode"
},
Expand Down Expand Up @@ -1211,9 +1214,6 @@
{
"name": "profiler"
},
{
"name": "provideZoneChangeDetection"
},
{
"name": "readableStreamLikeToAsyncGenerator"
},
Expand Down
12 changes: 3 additions & 9 deletions packages/core/test/bundling/router/bundle.golden_symbols.json
Expand Up @@ -1523,6 +1523,9 @@
{
"name": "internalImportProvidersFrom"
},
{
"name": "internalProvideZoneChangeDetection"
},
{
"name": "invokeDirectivesHostBindings"
},
Expand Down Expand Up @@ -1670,9 +1673,6 @@
{
"name": "lookupTokenUsingNodeInjector"
},
{
"name": "makeEnvironmentProviders"
},
{
"name": "makeRecord"
},
Expand Down Expand Up @@ -1817,9 +1817,6 @@
{
"name": "pathCompareMap"
},
{
"name": "performanceMarkFeature"
},
{
"name": "pipeFromArray"
},
Expand All @@ -1844,9 +1841,6 @@
{
"name": "profiler"
},
{
"name": "provideZoneChangeDetection"
},
{
"name": "readableStreamLikeToAsyncGenerator"
},
Expand Down
Expand Up @@ -815,6 +815,9 @@
{
"name": "internalImportProvidersFrom"
},
{
"name": "internalProvideZoneChangeDetection"
},
{
"name": "invokeHostBindingsInCreationMode"
},
Expand Down Expand Up @@ -947,9 +950,6 @@
{
"name": "parseAndConvertBindingsForDefinition"
},
{
"name": "performanceMarkFeature"
},
{
"name": "processInjectorTypesWithProviders"
},
Expand All @@ -962,9 +962,6 @@
{
"name": "profiler"
},
{
"name": "provideZoneChangeDetection"
},
{
"name": "refreshContentQueries"
},
Expand Down
3 changes: 3 additions & 0 deletions packages/core/test/bundling/todo/bundle.golden_symbols.json
Expand Up @@ -944,6 +944,9 @@
{
"name": "getNgDirectiveDef"
},
{
"name": "getNgZoneOptions"
},
{
"name": "getNodeInjectable"
},
Expand Down
8 changes: 8 additions & 0 deletions packages/core/test/change_detection_scheduler_spec.ts
Expand Up @@ -64,6 +64,14 @@ describe('Angular with zoneless enabled', () => {
});
});

it('throws an error if used with zone provider', () => {
TestBed.configureTestingModule({providers: [provideZoneChangeDetection()]});

expect(() => TestBed.inject(NgZone)).toThrowError(
/NG0408: Invalid change detection configuration/,
);
});

describe('notifies scheduler', () => {
it('contributes to application stableness', async () => {
const val = signal('initial');
Expand Down

0 comments on commit 1d7bc27

Please sign in to comment.