From 66c2af79e7542e272eb037d470047d1d35383a7e Mon Sep 17 00:00:00 2001 From: Artur Androsovych Date: Sat, 22 May 2021 20:10:09 +0300 Subject: [PATCH] fix(store): do not run `Promise.then` within synchronous tests when decorating factory (#1753) --- CHANGELOG.md | 1 + bundlesize.config.json | 4 +- .../bundlesize.config.json | 2 +- .../src/decorator-injector-adapter.ts | 45 +++++++++++++++---- setupJest.ts | 7 +-- 5 files changed, 45 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e764ddd41..b079de984 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ $ npm install @ngxs/store@dev ``` - Fix: Do not re-use the global `Store` instance between different apps [#1740](https://github.com/ngxs/store/pull/1740) +- Fix: Do not run `Promise.then` within synchronous tests when decorating factory [#1753](https://github.com/ngxs/store/pull/1753) - Performance: Tree-shake errors and warnings [#1732](https://github.com/ngxs/store/pull/1732) - Performance: Router Plugin - Tree-shake `isAngularInTestMode()` [#1738](https://github.com/ngxs/store/pull/1738) - Performance: Tree-shake `isAngularInTestMode()` [#1739](https://github.com/ngxs/store/pull/1739) diff --git a/bundlesize.config.json b/bundlesize.config.json index 7e9c9946b..3e4524298 100644 --- a/bundlesize.config.json +++ b/bundlesize.config.json @@ -4,14 +4,14 @@ "path": "./@ngxs/store/fesm2015/ngxs-store-internals.js", "package": "@ngxs/store/internals", "target": "es2015", - "maxSize": "10.85KB", + "maxSize": "11.75KB", "compression": "none" }, { "path": "./@ngxs/store/fesm5/ngxs-store-internals.js", "package": "@ngxs/store/internals", "target": "es5", - "maxSize": "11.80KB", + "maxSize": "12.70KB", "compression": "none" }, { diff --git a/integrations/hello-world-ng12-ivy/bundlesize.config.json b/integrations/hello-world-ng12-ivy/bundlesize.config.json index c1282d6c9..7873dc5c6 100644 --- a/integrations/hello-world-ng12-ivy/bundlesize.config.json +++ b/integrations/hello-world-ng12-ivy/bundlesize.config.json @@ -3,7 +3,7 @@ { "path": "./dist-integration/main.*.js", "target": "es2015", - "maxSize": "235.72 kB", + "maxSize": "236 kB", "compression": "none" } ] diff --git a/packages/store/internals/src/decorator-injector-adapter.ts b/packages/store/internals/src/decorator-injector-adapter.ts index 7caff8392..0078ff25d 100644 --- a/packages/store/internals/src/decorator-injector-adapter.ts +++ b/packages/store/internals/src/decorator-injector-adapter.ts @@ -1,4 +1,15 @@ -import { InjectionToken, Injector, INJECTOR, Type, ɵɵdirectiveInject } from '@angular/core'; +import { + InjectionToken, + Injector, + INJECTOR, + Type, + ɵɵdirectiveInject, + ɵglobal +} from '@angular/core'; + +// Will be provided through Terser global definitions by Angular CLI +// during the production build. This is how Angular does tree-shaking internally. +declare const ngDevMode: boolean; // Angular doesn't export `NG_FACTORY_DEF`. const NG_FACTORY_DEF = 'ɵfac'; @@ -19,16 +30,14 @@ export function ensureLocalInjectorCaptured(target: Object): void { // Means we're in AOT mode. if (typeof constructor[NG_FACTORY_DEF] === 'function') { decorateFactory(constructor); - } else { + } else if (ngDevMode) { // We're running in JIT mode and that means we're not able to get the compiled definition // on the class inside the property decorator during the current message loop tick. We have // to wait for the next message loop tick. Note that this is safe since this Promise will be // resolved even before the `APP_INITIALIZER` is resolved. // The below code also will be executed only in development mode, since it's never recommended // to use the JIT compiler in production mode (by setting "aot: false"). - Promise.resolve().then(() => { - decorateFactory(constructor); - }); + decorateFactoryLater(constructor); } target.constructor.prototype[FactoryHasBeenDecorated] = true; @@ -82,12 +91,32 @@ function decorateFactory(constructor: ConstructorWithDefinitionAndFactory): void } } -// We could've used `ɵɵFactoryDef` but we try to be backward compatible, +function decorateFactoryLater(constructor: ConstructorWithDefinitionAndFactory): void { + // This function actually will be tree-shaken away when building for production since it's guarded with `ngDevMode`. + // We're having the `try-catch` here because of the `SyncTestZoneSpec`, which throws + // an error when micro or macrotask is used within a synchronous test. E.g. `Cannot call + // Promise.then from within a sync test`. + try { + Promise.resolve().then(() => { + decorateFactory(constructor); + }); + } catch { + // This is kind of a "hack", but we try to be backwards-compatible, + // tho this `catch` block will only be executed when tests are run with Jasmine or Jest. + ɵglobal.process && + ɵglobal.process.nextTick && + ɵglobal.process.nextTick(() => { + decorateFactory(constructor); + }); + } +} + +// We could've used `ɵɵFactoryDef` but we try to be backwards-compatible, // since it's not exported in older Angular versions. type Factory = () => PrivateInstance; -// We could've used `ɵɵInjectableDef`, `ɵɵPipeDef`, etc. We try to be backward -// compatible since they're not exported in older Angular versions. +// We could've used `ɵɵInjectableDef`, `ɵɵPipeDef`, etc. We try to be backwards-compatible +// since they're not exported in older Angular versions. interface Definition { factory: Factory | null; } diff --git a/setupJest.ts b/setupJest.ts index ace41e02a..47e583bab 100644 --- a/setupJest.ts +++ b/setupJest.ts @@ -1,4 +1,5 @@ // import 'jest-preset-angular'; // commented out due to issue in latest jest-preset-angular +import { ɵglobal } from '@angular/core'; const CI = process.env['CI'] === 'true'; @@ -48,6 +49,6 @@ if (CI) { }); } -// Most of our tests are not aware of the `ngDevMode` Ivy and still rely on the old -// behavior (as it was in the View Engine). -((global as unknown) as { ngDevMode: undefined }).ngDevMode = undefined; +if (typeof ɵglobal.ngDevMode === 'undefined') { + ɵglobal.ngDevMode = true; +}