diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a2d9f8d7d3a..a18ff9814e7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -277,7 +277,7 @@ jobs: - name: Notify the slack channel of when build conclusion failed env: SLACK_BOT_TOKEN: ${{ secrets.SLACK_TOKEN }} - uses: slackapi/slack-github-action@v1.26.0 + uses: slackapi/slack-github-action@v1.27.0 with: channel-id: ${{ secrets.SLACK_NOTIFICATION_CHANNEL }} payload: | diff --git a/core-libs/setup/package.json b/core-libs/setup/package.json index de48148ca93..84a5eeffcdf 100644 --- a/core-libs/setup/package.json +++ b/core-libs/setup/package.json @@ -1,6 +1,6 @@ { "name": "@spartacus/setup", - "version": "2211.29.0", + "version": "2211.29.1", "description": "Includes features that makes Spartacus and it's setup easier and streamlined.", "keywords": [ "spartacus", @@ -21,10 +21,10 @@ "peerDependencies": { "@angular/core": "^17.0.5", "@angular/ssr": "^17.0.5", - "@spartacus/cart": "2211.29.0", - "@spartacus/core": "2211.29.0", - "@spartacus/order": "2211.29.0", - "@spartacus/user": "2211.29.0" + "@spartacus/cart": "2211.29.1", + "@spartacus/core": "2211.29.1", + "@spartacus/order": "2211.29.1", + "@spartacus/user": "2211.29.1" }, "optionalDependencies": { "@angular/platform-server": "^17.0.5", diff --git a/core-libs/setup/ssr/optimized-engine/optimized-ssr-engine.spec.ts b/core-libs/setup/ssr/optimized-engine/optimized-ssr-engine.spec.ts index 26fb3c402cd..18c25b9c4c9 100644 --- a/core-libs/setup/ssr/optimized-engine/optimized-ssr-engine.spec.ts +++ b/core-libs/setup/ssr/optimized-engine/optimized-ssr-engine.spec.ts @@ -36,14 +36,14 @@ class MockExpressServerLogger implements Partial { * Usage: * 1. Instantiate the class with desired options * 2. Call request() to run request through engine - * 3. Examine renders property for the renders + * 3. Examine responses property */ class TestEngineRunner { /** Accumulates html output for engine runs */ - renders: (string | Error)[] = []; + responses: (string | Error)[] = []; - /** Accumulates response parameters for engine runs */ - responseParams: object[] = []; + /** Accumulates responses' headers for engine runs */ + responsesHeaders: object[] = []; renderCount = 0; optimizedSsrEngine: OptimizedSsrEngine; @@ -86,7 +86,7 @@ class TestEngineRunner { return new TestEngineRunner(options, renderTime, { withError: true }); } - /** Run request against the engine. The result will be stored in rendering property. */ + /** Run request against the engine. The result will be stored in `responses` property. */ request( url: string, params?: { @@ -94,8 +94,8 @@ class TestEngineRunner { httpHeaders?: IncomingHttpHeaders; } ): TestEngineRunner { - const response: { [key: string]: string } = {}; - const headers = params?.httpHeaders ?? { host }; + const responseHeaders: { [key: string]: string } = {}; + const requestHeaders = params?.httpHeaders ?? { host }; /** used when resolving getRequestUrl() and getRequestOrigin() */ const app = >{ get: @@ -108,22 +108,22 @@ class TestEngineRunner { req: >{ protocol: 'https', originalUrl: url, - headers, + headers: requestHeaders, get: (header: string): string | string[] | null | undefined => { - return headers[header]; + return requestHeaders[header]; }, app, connection: >{}, res: >{ - set: (key: string, value: any) => (response[key] = value), + set: (key: string, value: any) => (responseHeaders[key] = value), locals: {}, }, }, }; this.engineInstance(url, optionsMock, (error, html): void => { - this.renders.push(html ?? error ?? ''); - this.responseParams.push(response); + this.responses.push(html ?? error ?? ''); + this.responsesHeaders.push(responseHeaders); }); return this; @@ -188,7 +188,9 @@ describe('OptimizedSsrEngine', () => { context: { timestamp: '2023-01-01T00:00:00.000Z', options: { + cache: false, cacheSize: 3000, + ttl: undefined, concurrency: 10, timeout: 50, forcedSsrTimeout: 60000, @@ -198,6 +200,9 @@ describe('OptimizedSsrEngine', () => { logger: 'DefaultExpressServerLogger', shouldCacheRenderingResult: '({ options, entry }) => !(options.ssrFeatureToggles?.avoidCachingErrors === true &&\\n' + ' Boolean(entry.err))', + renderKeyResolver: 'function getRequestUrl(req) {\\n' + + ' return (0, express_request_origin_1.getRequestOrigin)(req) + req.originalUrl;\\n' + + '}', ssrFeatureToggles: { avoidCachingErrors: false } } } @@ -208,12 +213,12 @@ describe('OptimizedSsrEngine', () => { }); describe('rendering', () => { - it('should return rendered output if no errors', fakeAsync(() => { + it('should return rendered HTML if no errors', fakeAsync(() => { const originalUrl = 'a'; const engineRunner = new TestEngineRunner({}).request('a'); tick(200); - expect(engineRunner.renders).toEqual(['a-0']); + expect(engineRunner.responses).toEqual(['a-0']); expect(consoleLogSpy).toHaveBeenCalledWith( expect.stringContaining( `Request is resolved with the SSR rendering result (${originalUrl})` @@ -226,7 +231,7 @@ describe('OptimizedSsrEngine', () => { const engineRunner = TestEngineRunner.withError({}).request('a'); tick(200); - expect(engineRunner.renders).toEqual([new Error('a-0')]); + expect(engineRunner.responses).toEqual([new Error('a-0')]); expect(consoleErrorSpy).toHaveBeenCalledWith( expect.stringContaining( `Request is resolved with the SSR rendering error (${originalUrl})` @@ -260,39 +265,39 @@ describe('OptimizedSsrEngine', () => { }); describe('timeout option', () => { - it('should fallback to CSR if rendering exceeds timeout', fakeAsync(() => { + it('should fallback to CSR if rendering exceeds a request timeout', fakeAsync(() => { const engineRunner = new TestEngineRunner({ timeout: 50 }).request('a'); tick(200); - expect(engineRunner.renders).toEqual(['']); + expect(engineRunner.responses).toEqual(['']); })); - it('should return timed out render in the followup request', fakeAsync(() => { + it('should reuse HTML meant for a previous timeouted request, if the new request is for the same url', fakeAsync(() => { const engineRunner = new TestEngineRunner({ timeout: 50 }).request('a'); tick(200); - expect(engineRunner.renders).toEqual(['']); + expect(engineRunner.responses).toEqual(['']); engineRunner.request('a'); - expect(engineRunner.renders[1]).toEqual('a-0'); + expect(engineRunner.responses[1]).toEqual('a-0'); })); - it('should return render if rendering meets timeout', fakeAsync(() => { + it('should return HTML if rendering meets timeout', fakeAsync(() => { const engineRunner = new TestEngineRunner({ timeout: 150 }).request('a'); tick(200); - expect(engineRunner.renders).toEqual(['a-0']); + expect(engineRunner.responses).toEqual(['a-0']); })); it('should fallback instantly if is set to 0', () => { const engineRunner = new TestEngineRunner({ timeout: 0 }).request('a'); - expect(engineRunner.renders).toEqual(['']); + expect(engineRunner.responses).toEqual(['']); }); - it('should return timed out render in the followup request, also when timeout is set to 0', fakeAsync(() => { + it('should return HTML meant for a previous timeouted request if the new request is for the same url, even if `timeout` is configured to 0', fakeAsync(() => { const engineRunner = new TestEngineRunner({ timeout: 0 }).request('a'); - expect(engineRunner.renders).toEqual(['']); + expect(engineRunner.responses).toEqual(['']); expect(getCurrentConcurrency(engineRunner)).toEqual({ currentConcurrency: 1, }); @@ -303,37 +308,38 @@ describe('OptimizedSsrEngine', () => { }); engineRunner.request('a'); - expect(engineRunner.renders[1]).toEqual('a-0'); + expect(engineRunner.responses[1]).toEqual('a-0'); expect(getCurrentConcurrency(engineRunner)).toEqual({ currentConcurrency: 0, }); })); }); - describe('no-store cache control header', () => { - it('should be applied for a fallback', () => { + describe('no-store cache control header in response', () => { + it('should be applied if a request times out ', () => { const engineRunner = new TestEngineRunner({ timeout: 0 }).request('a'); - expect(engineRunner.renders).toEqual(['']); - expect(engineRunner.responseParams).toEqual([ + expect(engineRunner.responses).toEqual(['']); + expect(engineRunner.responsesHeaders).toEqual([ { 'Cache-Control': 'no-store' }, ]); }); - it('should not be applied for a render within time limit', fakeAsync(() => { + it('should not be applied if rendering finishes before request times out', fakeAsync(() => { const engineRunner = new TestEngineRunner({ timeout: 200 }).request('a'); tick(200); - expect(engineRunner.renders).toEqual(['a-0']); - expect(engineRunner.responseParams).toEqual([{}]); + expect(engineRunner.responses).toEqual(['a-0']); + expect(engineRunner.responsesHeaders).toEqual([{}]); })); - it('should not be applied for a render served with next response', fakeAsync(() => { + + it('should not be applied for subsequent requests to the same url, when reusing a HTML meant for a previous timed out request', fakeAsync(() => { const engineRunner = new TestEngineRunner({ timeout: 50 }).request('a'); tick(200); engineRunner.request('a'); - expect(engineRunner.renders).toEqual(['', 'a-0']); - expect(engineRunner.responseParams).toEqual([ + expect(engineRunner.responses).toEqual(['', 'a-0']); + expect(engineRunner.responsesHeaders).toEqual([ { 'Cache-Control': 'no-store' }, {}, ]); @@ -341,7 +347,7 @@ describe('OptimizedSsrEngine', () => { }); describe('cache option', () => { - it('should not cache requests if disabled', fakeAsync(() => { + it('should not cache HTML if `cache` is disabled', fakeAsync(() => { const engineRunner = new TestEngineRunner({ cache: false, timeout: 200, @@ -352,9 +358,10 @@ describe('OptimizedSsrEngine', () => { tick(200); engineRunner.request('a'); tick(200); - expect(engineRunner.renders).toEqual(['a-0', 'a-1', 'a-2']); + expect(engineRunner.responses).toEqual(['a-0', 'a-1', 'a-2']); })); - it('should cache requests if enabled', fakeAsync(() => { + + it('should cache HTML if `cache` is enabled', fakeAsync(() => { const engineRunner = new TestEngineRunner({ cache: true, timeout: 200, @@ -379,7 +386,7 @@ describe('OptimizedSsrEngine', () => { tick(200); - expect(engineRunner.renders).toEqual(['a-0', 'a-0', 'a-0']); + expect(engineRunner.responses).toEqual(['a-0', 'a-0', 'a-0']); })); }); @@ -398,7 +405,7 @@ describe('OptimizedSsrEngine', () => { tick(200); engineRunner.request('a'); tick(200); - expect(engineRunner.renders).toEqual([ + expect(engineRunner.responses).toEqual([ new Error('a-0'), new Error('a-1'), new Error('a-2'), @@ -418,7 +425,7 @@ describe('OptimizedSsrEngine', () => { tick(200); engineRunner.request('a'); tick(200); - expect(engineRunner.renders).toEqual([ + expect(engineRunner.responses).toEqual([ new Error('a-0'), new Error('a-0'), new Error('a-0'), @@ -438,7 +445,7 @@ describe('OptimizedSsrEngine', () => { tick(200); engineRunner.request('a'); tick(200); - expect(engineRunner.renders).toEqual(['a-0', 'a-0', 'a-0']); + expect(engineRunner.responses).toEqual(['a-0', 'a-0', 'a-0']); })); it('should cache HTML if `avoidCachingErrors` is set to false', fakeAsync(() => { @@ -454,7 +461,7 @@ describe('OptimizedSsrEngine', () => { tick(200); engineRunner.request('a'); tick(200); - expect(engineRunner.renders).toEqual(['a-0', 'a-0', 'a-0']); + expect(engineRunner.responses).toEqual(['a-0', 'a-0', 'a-0']); })); }); }); @@ -471,7 +478,7 @@ describe('OptimizedSsrEngine', () => { tick(200); engineRunner.request('a'); tick(200); - expect(engineRunner.renders).toEqual([ + expect(engineRunner.responses).toEqual([ new Error('a-0'), new Error('a-1'), new Error('a-2'), @@ -489,7 +496,7 @@ describe('OptimizedSsrEngine', () => { tick(200); engineRunner.request('a'); tick(200); - expect(engineRunner.renders).toEqual([ + expect(engineRunner.responses).toEqual([ new Error('a-0'), new Error('a-0'), new Error('a-0'), @@ -507,7 +514,7 @@ describe('OptimizedSsrEngine', () => { tick(200); engineRunner.request('a'); tick(200); - expect(engineRunner.renders).toEqual(['a-0', 'a-1', 'a-2']); + expect(engineRunner.responses).toEqual(['a-0', 'a-1', 'a-2']); })); it('should cache HTML if `shouldCacheRenderingResult` returns true', fakeAsync(() => { @@ -521,7 +528,7 @@ describe('OptimizedSsrEngine', () => { tick(200); engineRunner.request('a'); tick(200); - expect(engineRunner.renders).toEqual(['a-0', 'a-0', 'a-0']); + expect(engineRunner.responses).toEqual(['a-0', 'a-0', 'a-0']); })); }); @@ -538,7 +545,7 @@ describe('OptimizedSsrEngine', () => { .request('e'); tick(200); - expect(engineRunner.renders).toEqual([ + expect(engineRunner.responses).toEqual([ '', // CSR fallback for 'd' '', // CSR fallback for 'e' 'a-0', @@ -561,7 +568,7 @@ describe('OptimizedSsrEngine', () => { engineRunner.request('f').request('g'); tick(200); - expect(engineRunner.renders).toEqual([ + expect(engineRunner.responses).toEqual([ '', // CSR fallback for 'c' 'a-0', '', // CSR fallback for 'e' @@ -574,7 +581,7 @@ describe('OptimizedSsrEngine', () => { }); describe('ttl option', () => { - it('should invalidate expired renders', fakeAsync(() => { + it('should invalidate expired cache entries', fakeAsync(() => { let currentDate = 100; jest.spyOn(Date, 'now').mockImplementation(() => currentDate); @@ -593,10 +600,10 @@ describe('OptimizedSsrEngine', () => { engineRunner.request('a'); tick(200); - expect(engineRunner.renders).toEqual(['a-0', 'a-0', 'a-1']); + expect(engineRunner.responses).toEqual(['a-0', 'a-0', 'a-1']); })); - it('should not invalidate renders if ttl is not defined', fakeAsync(() => { + it('should not invalidate cache entries if `ttl` is not defined', fakeAsync(() => { let currentDate = 100; jest.spyOn(Date, 'now').mockImplementation(() => currentDate); @@ -614,7 +621,7 @@ describe('OptimizedSsrEngine', () => { engineRunner.request('a'); tick(200); - expect(engineRunner.renders).toEqual(['a-0', 'a-0', 'a-0']); + expect(engineRunner.responses).toEqual(['a-0', 'a-0', 'a-0']); })); }); @@ -679,7 +686,7 @@ describe('OptimizedSsrEngine', () => { tick(200); engineRunner.request('elu'); tick(200); - expect(engineRunner.renders).toEqual([ + expect(engineRunner.responses).toEqual([ 'ala-0', 'ala-0', 'ela-1', @@ -699,7 +706,7 @@ describe('OptimizedSsrEngine', () => { }).request('a'); tick(200); - expect(engineRunner.renders).toEqual(['a-0']); + expect(engineRunner.responses).toEqual(['a-0']); })); it('should ignore timeout also when it is set to 0', fakeAsync(() => { @@ -712,10 +719,10 @@ describe('OptimizedSsrEngine', () => { engineRunner.request('a'); tick(200); - expect(engineRunner.renders).toEqual(['a-0']); + expect(engineRunner.responses).toEqual(['a-0']); })); - it('when reuseCurrentRendering is false, it should render each request separately, even if there is already a pending render for the same rendering key', fakeAsync(() => { + it('when reuseCurrentRendering is false, it should render for each request separately, even if there is already a pending render for the same rendering key', fakeAsync(() => { const engineRunner = new TestEngineRunner({ renderingStrategyResolver: () => RenderingStrategy.ALWAYS_SSR, timeout: 200, @@ -736,10 +743,10 @@ describe('OptimizedSsrEngine', () => { expect(getCurrentConcurrency(engineRunner)).toEqual({ currentConcurrency: 2, }); - expect(engineRunner.renders).toEqual([]); + expect(engineRunner.responses).toEqual([]); tick(100); - expect(engineRunner.renders).toEqual(['a-0', 'a-1']); + expect(engineRunner.responses).toEqual(['a-0', 'a-1']); expect( engineRunner.optimizedSsrEngine['expressEngine'] ).toHaveBeenCalledTimes(2); @@ -762,7 +769,7 @@ describe('OptimizedSsrEngine', () => { tick(200); engineRunner.request('a'); tick(200); - expect(engineRunner.renders).toEqual(['', '']); + expect(engineRunner.responses).toEqual(['', '']); })); it('should not start the actual render in the background', fakeAsync(() => { @@ -777,7 +784,7 @@ describe('OptimizedSsrEngine', () => { ); engineRunner.request('a'); - expect(engineRunner.renders).toEqual(['']); + expect(engineRunner.responses).toEqual(['']); expect( engineRunner.optimizedSsrEngine['expressEngine'] @@ -796,10 +803,10 @@ describe('OptimizedSsrEngine', () => { tick(200); engineRunner.request('a'); - expect(engineRunner.renders).toEqual(['', 'a-0']); + expect(engineRunner.responses).toEqual(['', 'a-0']); })); - it('when reuseCurrentRendering is false, it should fallback to CSR when there is already pending a render for the same rendering key', fakeAsync(() => { + it('when reuseCurrentRendering is false, it should fallback to CSR when there is already a pending render for the same rendering key', fakeAsync(() => { const engineRunner = new TestEngineRunner({ renderingStrategyResolver: () => RenderingStrategy.DEFAULT, timeout: 200, @@ -815,10 +822,10 @@ describe('OptimizedSsrEngine', () => { currentConcurrency: 1, }); - expect(engineRunner.renders).toEqual(['']); // immediate fallback to CSR for the 2nd request for the same key + expect(engineRunner.responses).toEqual(['']); // immediate fallback to CSR for the 2nd request for the same key tick(100); - expect(engineRunner.renders).toEqual(['', 'a-0']); + expect(engineRunner.responses).toEqual(['', 'a-0']); expect(getCurrentConcurrency(engineRunner)).toEqual({ currentConcurrency: 0, }); @@ -839,13 +846,13 @@ describe('OptimizedSsrEngine', () => { engineRunner.request('a', { httpHeaders: { 'User-Agent': 'bot' } }); tick(200); - expect(engineRunner.renders).toEqual(['', 'a-1']); + expect(engineRunner.responses).toEqual(['', 'a-1']); })); }); }); describe('forcedSsrTimeout option', () => { - it('should fallback to CSR when forcedSsrTimeout timeout is exceeded for ALWAYS_SSR rendering strategy, and return the timed out render in the followup request', fakeAsync(() => { + it('should fallback to CSR when forcedSsrTimeout timeout is exceeded for ALWAYS_SSR rendering strategy, and after that rendering ends, its HTML should be reused in the next request for the same url', fakeAsync(() => { const engineRunner = new TestEngineRunner({ renderingStrategyResolver: () => RenderingStrategy.ALWAYS_SSR, timeout: 50, @@ -858,15 +865,15 @@ describe('OptimizedSsrEngine', () => { }); tick(60); - expect(engineRunner.renders).toEqual([]); + expect(engineRunner.responses).toEqual([]); tick(50); - expect(engineRunner.renders).toEqual(['']); + expect(engineRunner.responses).toEqual(['']); expect(getCurrentConcurrency(engineRunner)).toEqual({ currentConcurrency: 0, }); engineRunner.request('a'); - expect(engineRunner.renders).toEqual(['', 'a-0']); + expect(engineRunner.responses).toEqual(['', 'a-0']); expect(getCurrentConcurrency(engineRunner)).toEqual({ currentConcurrency: 0, }); @@ -881,11 +888,11 @@ describe('OptimizedSsrEngine', () => { engineRunner.request('a'); tick(60); - expect(engineRunner.renders).toEqual(['']); + expect(engineRunner.responses).toEqual(['']); tick(50); engineRunner.request('a'); - expect(engineRunner.renders).toEqual(['', 'a-0']); + expect(engineRunner.responses).toEqual(['', 'a-0']); })); }); describe('maxRenderTime option', () => { @@ -1017,7 +1024,7 @@ describe('OptimizedSsrEngine', () => { renderTime ).request(requestUrl); jest.spyOn(engineRunner.optimizedSsrEngine as any, 'log'); - expect(engineRunner.renders).toEqual([]); + expect(engineRunner.responses).toEqual([]); tick(fiveMinutes + 101); expect(engineRunner.optimizedSsrEngine['log']).toHaveBeenCalledWith( @@ -1025,10 +1032,10 @@ describe('OptimizedSsrEngine', () => { false, { request: expect.objectContaining({ originalUrl: requestUrl }) } ); - expect(engineRunner.renders).toEqual(['']); + expect(engineRunner.responses).toEqual(['']); engineRunner.request(requestUrl); - expect(engineRunner.renders).toEqual(['']); // if the result was cached, the 2nd request would get immediately 'a-0' + expect(engineRunner.responses).toEqual(['']); // if the result was cached, the 2nd request would get immediately 'a-0' flush(); })); }); @@ -1083,14 +1090,14 @@ describe('OptimizedSsrEngine', () => { false, { request: expect.objectContaining({ originalUrl: requestUrl }) } ); - expect(engineRunner.renders).toEqual(['', '']); + expect(engineRunner.responses).toEqual(['', '']); flush(); })); }); describe('when enabled', () => { - describe('multiple subsequent requests for the same rendering key should reuse the same render', () => { + describe('multiple subsequent requests for the same rendering key should await for the result of the same pending rendering, and all get the same HTML response', () => { it('and the first request should timeout', fakeAsync(() => { const timeout = 300; const engineRunner = new TestEngineRunner( @@ -1113,7 +1120,7 @@ describe('OptimizedSsrEngine', () => { tick(100); expect(engineRunner.renderCount).toEqual(1); - expect(engineRunner.renders).toEqual(['', `${requestUrl}-0`]); + expect(engineRunner.responses).toEqual(['', `${requestUrl}-0`]); flush(); })); @@ -1151,7 +1158,7 @@ describe('OptimizedSsrEngine', () => { expect(renderExceedMessageCount).toBe(2); expect(engineRunner.renderCount).toEqual(0); - expect(engineRunner.renders).toEqual(['', '']); + expect(engineRunner.responses).toEqual(['', '']); flush(); })); @@ -1187,7 +1194,7 @@ describe('OptimizedSsrEngine', () => { tick(200); expect(engineRunner.renderCount).toEqual(1); - expect(engineRunner.renders).toEqual([ + expect(engineRunner.responses).toEqual([ `${requestUrl}-0`, `${requestUrl}-0`, ]); @@ -1237,7 +1244,7 @@ describe('OptimizedSsrEngine', () => { false, { request: expect.objectContaining({ originalUrl: requestUrl }) } ); - expect(engineRunner.renders).toEqual(['']); // the first request fallback to CSR due to timeout + expect(engineRunner.responses).toEqual(['']); // the first request fallback to CSR due to timeout expect(getCurrentConcurrency(engineRunner)).toEqual({ currentConcurrency: 1, }); // the render still continues in the background @@ -1245,7 +1252,7 @@ describe('OptimizedSsrEngine', () => { // eventually the render succeeds and 2 remaining requests get the same response: tick(100); expect(engineRunner.renderCount).toEqual(1); - expect(engineRunner.renders).toEqual([ + expect(engineRunner.responses).toEqual([ '', // CSR fallback of the 1st request due to it timed out `${requestUrl}-0`, `${requestUrl}-0`, @@ -1260,7 +1267,7 @@ describe('OptimizedSsrEngine', () => { flush(); })); - it('and concurrency limit should NOT fallback to CSR, when the request is for a pending render', fakeAsync(() => { + it('and concurrency limit should NOT fallback to CSR, when there is a pending rendering for the same rendering key', fakeAsync(() => { const engineRunner = new TestEngineRunner({ reuseCurrentRendering: true, timeout: 200, @@ -1277,7 +1284,7 @@ describe('OptimizedSsrEngine', () => { ).not.toHaveBeenCalledWith( `CSR fallback: Concurrency limit exceeded (1)` ); - expect(engineRunner.renders).toEqual(['a-0', 'a-0']); + expect(engineRunner.responses).toEqual(['a-0', 'a-0']); })); it('combined with a different request should take up two concurrency slots', fakeAsync(() => { @@ -1312,7 +1319,7 @@ describe('OptimizedSsrEngine', () => { }); tick(250); - expect(engineRunner.renders).toEqual([ + expect(engineRunner.responses).toEqual([ 'a-0', 'a-0', 'a-0', @@ -1429,7 +1436,7 @@ describe('OptimizedSsrEngine', () => { ); expect(engineRunner.renderCount).toEqual(1); - expect(engineRunner.renders).toEqual(['', '']); + expect(engineRunner.responses).toEqual(['', '']); flush(); })); @@ -1459,34 +1466,39 @@ describe('OptimizedSsrEngine', () => { logger: new MockExpressServerLogger() as ExpressServerLogger, }); expect(consoleLogSpy.mock.lastCall).toMatchInlineSnapshot(` - [ - "[spartacus] SSR optimization engine initialized", - { - "options": { - "cacheSize": 3000, - "concurrency": 10, - "forcedSsrTimeout": 60000, - "logger": "MockExpressServerLogger", - "maxRenderTime": 300000, - "renderingStrategyResolver": "(request) => { - if (hasExcludedUrl(request, defaultAlwaysCsrOptions.excludedUrls)) { - return ssr_optimization_options_1.RenderingStrategy.ALWAYS_CSR; - } - return shouldFallbackToCsr(request, options) - ? ssr_optimization_options_1.RenderingStrategy.ALWAYS_CSR - : ssr_optimization_options_1.RenderingStrategy.DEFAULT; - }", - "reuseCurrentRendering": true, - "shouldCacheRenderingResult": "({ options, entry }) => !(options.ssrFeatureToggles?.avoidCachingErrors === true && - Boolean(entry.err))", - "ssrFeatureToggles": { - "avoidCachingErrors": false, - }, - "timeout": 3000, - }, - }, - ] - `); +[ + "[spartacus] SSR optimization engine initialized", + { + "options": { + "cache": false, + "cacheSize": 3000, + "concurrency": 10, + "forcedSsrTimeout": 60000, + "logger": "MockExpressServerLogger", + "maxRenderTime": 300000, + "renderKeyResolver": "function getRequestUrl(req) { + return (0, express_request_origin_1.getRequestOrigin)(req) + req.originalUrl; +}", + "renderingStrategyResolver": "(request) => { + if (hasExcludedUrl(request, defaultAlwaysCsrOptions.excludedUrls)) { + return ssr_optimization_options_1.RenderingStrategy.ALWAYS_CSR; + } + return shouldFallbackToCsr(request, options) + ? ssr_optimization_options_1.RenderingStrategy.ALWAYS_CSR + : ssr_optimization_options_1.RenderingStrategy.DEFAULT; +}", + "reuseCurrentRendering": true, + "shouldCacheRenderingResult": "({ options, entry }) => !(options.ssrFeatureToggles?.avoidCachingErrors === true && + Boolean(entry.err))", + "ssrFeatureToggles": { + "avoidCachingErrors": false, + }, + "timeout": 3000, + "ttl": undefined, + }, + }, +] +`); }); }); }); diff --git a/core-libs/setup/ssr/optimized-engine/optimized-ssr-engine.ts b/core-libs/setup/ssr/optimized-engine/optimized-ssr-engine.ts index 148373f0313..66d14fe1896 100644 --- a/core-libs/setup/ssr/optimized-engine/optimized-ssr-engine.ts +++ b/core-libs/setup/ssr/optimized-engine/optimized-ssr-engine.ts @@ -8,7 +8,6 @@ import { Request, Response } from 'express'; import * as fs from 'fs'; import { NgExpressEngineInstance } from '../engine-decorator/ng-express-engine-decorator'; -import { getRequestUrl } from '../express-utils/express-request-url'; import { EXPRESS_SERVER_LOGGER, ExpressServerLogger, @@ -21,13 +20,9 @@ import { RenderingStrategy, SsrOptimizationOptions, defaultSsrOptimizationOptions, + getDefaultRenderKey, } from './ssr-optimization-options'; -/** - * Returns the full url for the given SSR Request. - */ -export const getDefaultRenderKey = getRequestUrl; - export type SsrCallbackFn = ( /** * Error that might've occurred while rendering. diff --git a/core-libs/setup/ssr/optimized-engine/ssr-optimization-options.ts b/core-libs/setup/ssr/optimized-engine/ssr-optimization-options.ts index 9f3610ef5da..6508f5044f0 100644 --- a/core-libs/setup/ssr/optimized-engine/ssr-optimization-options.ts +++ b/core-libs/setup/ssr/optimized-engine/ssr-optimization-options.ts @@ -5,6 +5,7 @@ */ import { Request } from 'express'; +import { getRequestUrl } from '../express-utils/express-request-url'; import { DefaultExpressServerLogger, ExpressServerLogger } from '../logger'; import { RenderingEntry } from './rendering-cache.model'; import { defaultRenderingStrategyResolver } from './rendering-strategy-resolver'; @@ -48,7 +49,7 @@ export interface SsrOptimizationOptions { * Time in milliseconds after prerendered page is becoming stale and should * be rendered again. */ - ttl?: number; + ttl?: number | undefined; /** * Allows overriding default key generator for custom differentiating @@ -188,10 +189,34 @@ type DeepRequired = { [P in keyof T]-?: DeepRequired; }; -export const defaultSsrOptimizationOptions: SsrOptimizationOptions & - // To not forget adding default values, when adding new feature toggles in the type in the future - DeepRequired> = { +/** + * Returns the full url for the given SSR Request. + */ +export const getDefaultRenderKey = getRequestUrl; + +/** + * The type of `defaultSsrOptimizationOptions` ensures that all properties are set to a default value. + * Thanks to this, all options are well-defined and can be printed to logs on the SSR server start. + */ +type DefaultSsrOptimizationOptions = Omit< + Required, + | 'debug' // debug is deprecated and not used anymore + | 'ttl' // ttl is required but its default value is `undefined` +> & { + ttl: number | undefined; // needed, otherwise we could not set the value `ttl: undefined` value (due to the Required<...>) +} & DeepRequired< + // all nested properties of `ssrFeatureToggles` are required too + Pick< + SsrOptimizationOptions, + // + 'ssrFeatureToggles' + > + >; + +export const defaultSsrOptimizationOptions: DefaultSsrOptimizationOptions = { + cache: false, cacheSize: 3000, + ttl: undefined, concurrency: 10, timeout: 3_000, forcedSsrTimeout: 60_000, @@ -206,6 +231,7 @@ export const defaultSsrOptimizationOptions: SsrOptimizationOptions & options.ssrFeatureToggles?.avoidCachingErrors === true && Boolean(entry.err) ), + renderKeyResolver: getDefaultRenderKey, ssrFeatureToggles: { avoidCachingErrors: false, }, diff --git a/feature-libs/asm/package.json b/feature-libs/asm/package.json index 605268eff92..9574a9aa1e4 100644 --- a/feature-libs/asm/package.json +++ b/feature-libs/asm/package.json @@ -1,6 +1,6 @@ { "name": "@spartacus/asm", - "version": "2211.29.0", + "version": "2211.29.1", "description": "ASM feature library for Spartacus", "keywords": [ "spartacus", @@ -32,14 +32,14 @@ "@ng-select/ng-select": "^12.0.4", "@ngrx/effects": "^17.0.1", "@ngrx/store": "^17.0.1", - "@spartacus/cart": "2211.29.0", - "@spartacus/core": "2211.29.0", - "@spartacus/order": "2211.29.0", - "@spartacus/schematics": "2211.29.0", - "@spartacus/storefinder": "2211.29.0", - "@spartacus/storefront": "2211.29.0", - "@spartacus/styles": "2211.29.0", - "@spartacus/user": "2211.29.0", + "@spartacus/cart": "2211.29.1", + "@spartacus/core": "2211.29.1", + "@spartacus/order": "2211.29.1", + "@spartacus/schematics": "2211.29.1", + "@spartacus/storefinder": "2211.29.1", + "@spartacus/storefront": "2211.29.1", + "@spartacus/styles": "2211.29.1", + "@spartacus/user": "2211.29.1", "rxjs": "^7.8.0" }, "publishConfig": { diff --git a/feature-libs/cart/package.json b/feature-libs/cart/package.json index 8e5caa530c0..7412f1aa97d 100644 --- a/feature-libs/cart/package.json +++ b/feature-libs/cart/package.json @@ -1,6 +1,6 @@ { "name": "@spartacus/cart", - "version": "2211.29.0", + "version": "2211.29.1", "description": "", "keywords": [ "spartacus", @@ -37,11 +37,11 @@ "@ng-select/ng-select": "^12.0.4", "@ngrx/effects": "^17.0.1", "@ngrx/store": "^17.0.1", - "@spartacus/core": "2211.29.0", - "@spartacus/schematics": "2211.29.0", - "@spartacus/storefront": "2211.29.0", - "@spartacus/styles": "2211.29.0", - "@spartacus/user": "2211.29.0", + "@spartacus/core": "2211.29.1", + "@spartacus/schematics": "2211.29.1", + "@spartacus/storefront": "2211.29.1", + "@spartacus/styles": "2211.29.1", + "@spartacus/user": "2211.29.1", "bootstrap": "^4.6.2", "rxjs": "^7.8.0" }, diff --git a/feature-libs/checkout/package.json b/feature-libs/checkout/package.json index 01d6895b6f4..9daf3db9284 100644 --- a/feature-libs/checkout/package.json +++ b/feature-libs/checkout/package.json @@ -1,6 +1,6 @@ { "name": "@spartacus/checkout", - "version": "2211.29.0", + "version": "2211.29.1", "description": "Checkout feature library for Spartacus", "keywords": [ "spartacus", @@ -32,13 +32,13 @@ "@angular/router": "^17.0.5", "@ng-select/ng-select": "^12.0.4", "@ngrx/store": "^17.0.1", - "@spartacus/cart": "2211.29.0", - "@spartacus/core": "2211.29.0", - "@spartacus/order": "2211.29.0", - "@spartacus/schematics": "2211.29.0", - "@spartacus/storefront": "2211.29.0", - "@spartacus/styles": "2211.29.0", - "@spartacus/user": "2211.29.0", + "@spartacus/cart": "2211.29.1", + "@spartacus/core": "2211.29.1", + "@spartacus/order": "2211.29.1", + "@spartacus/schematics": "2211.29.1", + "@spartacus/storefront": "2211.29.1", + "@spartacus/styles": "2211.29.1", + "@spartacus/user": "2211.29.1", "bootstrap": "^4.6.2", "rxjs": "^7.8.0" }, diff --git a/feature-libs/customer-ticketing/package.json b/feature-libs/customer-ticketing/package.json index 4acd04b0509..33e486a7fb7 100644 --- a/feature-libs/customer-ticketing/package.json +++ b/feature-libs/customer-ticketing/package.json @@ -1,6 +1,6 @@ { "name": "@spartacus/customer-ticketing", - "version": "2211.29.0", + "version": "2211.29.1", "description": "Customer-Ticketing library for Spartacus", "keywords": [ "spartacus", @@ -30,11 +30,11 @@ "@angular/core": "^17.0.5", "@angular/forms": "^17.0.5", "@angular/router": "^17.0.5", - "@spartacus/cart": "2211.29.0", - "@spartacus/core": "2211.29.0", - "@spartacus/schematics": "2211.29.0", - "@spartacus/storefront": "2211.29.0", - "@spartacus/styles": "2211.29.0", + "@spartacus/cart": "2211.29.1", + "@spartacus/core": "2211.29.1", + "@spartacus/schematics": "2211.29.1", + "@spartacus/storefront": "2211.29.1", + "@spartacus/styles": "2211.29.1", "rxjs": "^7.8.0" }, "publishConfig": { diff --git a/feature-libs/estimated-delivery-date/package.json b/feature-libs/estimated-delivery-date/package.json index e919c30b847..e18895481f0 100644 --- a/feature-libs/estimated-delivery-date/package.json +++ b/feature-libs/estimated-delivery-date/package.json @@ -1,6 +1,6 @@ { "name": "@spartacus/estimated-delivery-date", - "version": "2211.29.0", + "version": "2211.29.1", "description": "Estimated Delivery Date library for Spartacus", "keywords": [ "spartacus", @@ -28,12 +28,12 @@ "@angular-devkit/schematics": "^17.0.5", "@angular/common": "^17.0.5", "@angular/core": "^17.0.5", - "@spartacus/cart": "2211.29.0", - "@spartacus/core": "2211.29.0", - "@spartacus/order": "2211.29.0", - "@spartacus/schematics": "2211.29.0", - "@spartacus/storefront": "2211.29.0", - "@spartacus/styles": "2211.29.0", + "@spartacus/cart": "2211.29.1", + "@spartacus/core": "2211.29.1", + "@spartacus/order": "2211.29.1", + "@spartacus/schematics": "2211.29.1", + "@spartacus/storefront": "2211.29.1", + "@spartacus/styles": "2211.29.1", "rxjs": "^7.8.0" }, "publishConfig": { diff --git a/feature-libs/order/components/amend-order/amend-order-actions/amend-order-actions.component.html b/feature-libs/order/components/amend-order/amend-order-actions/amend-order-actions.component.html index c91e3835569..203a3e58fbb 100644 --- a/feature-libs/order/components/amend-order/amend-order-actions/amend-order-actions.component.html +++ b/feature-libs/order/components/amend-order/amend-order-actions/amend-order-actions.component.html @@ -1,5 +1,6 @@
{{ 'orderApprovalDetails.back' | cxTranslate }} diff --git a/feature-libs/pickup-in-store/components/container/cart-pickup-options-container/cart-pickup-options-container.component.spec.ts b/feature-libs/pickup-in-store/components/container/cart-pickup-options-container/cart-pickup-options-container.component.spec.ts index d2b4b220687..4a2d0d22a95 100644 --- a/feature-libs/pickup-in-store/components/container/cart-pickup-options-container/cart-pickup-options-container.component.spec.ts +++ b/feature-libs/pickup-in-store/components/container/cart-pickup-options-container/cart-pickup-options-container.component.spec.ts @@ -1,4 +1,5 @@ import { CommonModule } from '@angular/common'; +import { ElementRef } from '@angular/core'; import { ComponentFixture, TestBed, @@ -6,7 +7,12 @@ import { tick, } from '@angular/core/testing'; import { ActiveCartFacade, Cart, OrderEntry } from '@spartacus/cart/base/root'; -import { CmsService, I18nTestingModule, Page } from '@spartacus/core'; +import { + CmsService, + FeatureConfigService, + I18nTestingModule, + Page, +} from '@spartacus/core'; import { AugmentedPointOfService, IntendedPickupLocationFacade, @@ -16,8 +22,8 @@ import { PreferredStoreFacade, } from '@spartacus/pickup-in-store/root'; import { - LaunchDialogService, LAUNCH_CALLER, + LaunchDialogService, OutletContextData, } from '@spartacus/storefront'; import { cold } from 'jasmine-marbles'; @@ -109,6 +115,12 @@ const mockOutletContext: { item: OrderEntry; cartType: string } = { cartType: 'cart', }; +class MockFeatureConfigService { + isEnabled() { + return true; + } +} + const context$ = of(mockOutletContext); class MockIntendedPickupLocationFacade { @@ -167,6 +179,10 @@ describe('CartPickupOptionsContainerComponent', () => { provide: IntendedPickupLocationFacade, useClass: MockIntendedPickupLocationFacade, }, + { + provide: FeatureConfigService, + useClass: MockFeatureConfigService, + }, ], }); @@ -198,10 +214,11 @@ describe('CartPickupOptionsContainerComponent', () => { }); it('should trigger and open dialog', () => { - component.openDialog(); + const triggerElement = new ElementRef({}); + component.openDialog(triggerElement); expect(launchDialogService.openDialog).toHaveBeenCalledWith( LAUNCH_CALLER.PICKUP_IN_STORE, - component.element, + triggerElement, component['vcr'], { productCode: 'productCode1', entryNumber: 1, quantity: 1 } ); @@ -210,7 +227,12 @@ describe('CartPickupOptionsContainerComponent', () => { it('should not openDialog if display name is not set and ship it is selected', () => { spyOn(component, 'openDialog'); component['displayNameIsSet'] = false; - component.onPickupOptionChange('delivery'); + const pickupOption: PickupOption = 'delivery'; + const event = { + option: pickupOption, + triggerElement: new ElementRef({}), + }; + component.onPickupOptionChange(event); expect(component.openDialog).not.toHaveBeenCalled(); }); @@ -218,6 +240,10 @@ describe('CartPickupOptionsContainerComponent', () => { const entryNumber = 2; const pickupOption: PickupOption = 'pickup'; const quantity = 3; + const event = { + option: pickupOption, + triggerElement: new ElementRef({}), + }; component.entryNumber = entryNumber; component.quantity = quantity; @@ -226,7 +252,7 @@ describe('CartPickupOptionsContainerComponent', () => { spyOn(pickupOptionService, 'setPickupOption'); spyOn(activeCartService, 'updateEntry'); - component.onPickupOptionChange(pickupOption); + component.onPickupOptionChange(event); expect(pickupOptionService.setPickupOption).toHaveBeenCalledWith( entryNumber, pickupOption diff --git a/feature-libs/pickup-in-store/components/container/cart-pickup-options-container/cart-pickup-options-container.component.ts b/feature-libs/pickup-in-store/components/container/cart-pickup-options-container/cart-pickup-options-container.component.ts index ecfdee85fed..5abd5ec2cfc 100644 --- a/feature-libs/pickup-in-store/components/container/cart-pickup-options-container/cart-pickup-options-container.component.ts +++ b/feature-libs/pickup-in-store/components/container/cart-pickup-options-container/cart-pickup-options-container.component.ts @@ -7,6 +7,7 @@ import { Component, ElementRef, + inject, OnDestroy, OnInit, Optional, @@ -18,7 +19,7 @@ import { CartType, OrderEntry, } from '@spartacus/cart/base/root'; -import { CmsService, Page } from '@spartacus/core'; +import { CmsService, FeatureConfigService, Page } from '@spartacus/core'; import { cartWithIdAndUserId, getProperty, @@ -30,8 +31,8 @@ import { RequiredDeepPath, } from '@spartacus/pickup-in-store/root'; import { - LaunchDialogService, LAUNCH_CALLER, + LaunchDialogService, OutletContextData, } from '@spartacus/storefront'; import { EMPTY, iif, Observable, of, Subscription } from 'rxjs'; @@ -79,6 +80,12 @@ export function orderEntryWithRequiredFields( templateUrl: 'cart-pickup-options-container.component.html', }) export class CartPickupOptionsContainerComponent implements OnInit, OnDestroy { + // TODO: Remove element reference once 'a11yDialogTriggerRefocus' feature flag is removed. + /** + * @deprecated since 2211.28.0 + * This reference does not point to any element and will be removed at earliest convinience. + * The 'triggerElement' is passed through 'PickupOptionChange' event instead. + */ @ViewChild('open') element: ElementRef; pickupOption$: Observable; @@ -98,6 +105,7 @@ export class CartPickupOptionsContainerComponent implements OnInit, OnDestroy { private displayNameIsSet = false; page?: string; readonly CartType = CartType; + private featureConfigService = inject(FeatureConfigService); constructor( protected activeCartFacade: ActiveCartFacade, protected launchDialogService: LaunchDialogService, @@ -253,40 +261,92 @@ export class CartPickupOptionsContainerComponent implements OnInit, OnDestroy { tap((_) => (this.displayNameIsSet = true)) ); } - - onPickupOptionChange(pickupOption: PickupOption): void { - this.pickupOptionFacade.setPickupOption(this.entryNumber, pickupOption); - if (pickupOption === 'delivery') { - this.activeCartFacade.updateEntry( - this.entryNumber, - this.quantity, - undefined, - true - ); - return; - } - [pickupOption] - .filter((option) => option === 'pickup') - .forEach(() => { - this.subscription.add( - this.storeDetails$ - .pipe( - filter(({ name }) => !!name), - tap(({ name }) => - this.activeCartFacade.updateEntry( - this.entryNumber, - this.quantity, - name, - true + // TODO: Remove 'PickupOption' argument type once 'a11yDialogTriggerRefocus' feature flag is removed. + /** + * @deprecated since 2211.28.0 - Use event param instead of option. + * @param event - Object containing the selected option and the element that triggered the change. + */ + onPickupOptionChange(pickupOption: PickupOption): void; + // eslint-disable-next-line @typescript-eslint/unified-signatures + onPickupOptionChange(event: { + option: PickupOption; + triggerElement: ElementRef; + }): void; + onPickupOptionChange( + event: { option: PickupOption; triggerElement: ElementRef } | PickupOption + ): void { + /* istanbul ignore else */ + if ( + this.featureConfigService.isEnabled('a11yDialogTriggerRefocus') && + typeof event === 'object' + ) { + this.pickupOptionFacade.setPickupOption(this.entryNumber, event.option); + if (event.option === 'delivery') { + this.activeCartFacade.updateEntry( + this.entryNumber, + this.quantity, + undefined, + true + ); + return; + } + [event.option] + .filter((option) => option === 'pickup') + .forEach(() => { + this.subscription.add( + this.storeDetails$ + .pipe( + filter(({ name }) => !!name), + tap(({ name }) => + this.activeCartFacade.updateEntry( + this.entryNumber, + this.quantity, + name, + true + ) ) ) - ) - .subscribe() + .subscribe() + ); + }); + + if (!this.displayNameIsSet) { + this.openDialog(event.triggerElement); + } + } else if (typeof event === 'string') { + this.pickupOptionFacade.setPickupOption(this.entryNumber, event); + if (event === 'delivery') { + this.activeCartFacade.updateEntry( + this.entryNumber, + this.quantity, + undefined, + true ); - }); + return; + } + [event] + .filter((option) => option === 'pickup') + .forEach(() => { + this.subscription.add( + this.storeDetails$ + .pipe( + filter(({ name }) => !!name), + tap(({ name }) => + this.activeCartFacade.updateEntry( + this.entryNumber, + this.quantity, + name, + true + ) + ) + ) + .subscribe() + ); + }); - if (!this.displayNameIsSet) { - this.openDialog(); + if (!this.displayNameIsSet) { + this.openDialog(); + } } } @@ -294,10 +354,15 @@ export class CartPickupOptionsContainerComponent implements OnInit, OnDestroy { this.subscription.unsubscribe(); } - openDialog(): void { + // TODO: Make argument required once 'a11yDialogTriggerRefocus' feature flag is removed. + /** + * @deprecated since 2211.28.0 - The use of TriggerElement param will become mandatory. + * @param triggerElement - The reference of element that triggered the dialog. Used to refocus on it after the dialog is closed. + */ + openDialog(triggerElement?: ElementRef): void { const dialog = this.launchDialogService.openDialog( LAUNCH_CALLER.PICKUP_IN_STORE, - this.element, + triggerElement ? triggerElement : this.element, this.vcr, { productCode: this.productCode, diff --git a/feature-libs/pickup-in-store/components/presentational/store/store.component.html b/feature-libs/pickup-in-store/components/presentational/store/store.component.html index 92178a8f2bd..08a40fd8acf 100644 --- a/feature-libs/pickup-in-store/components/presentational/store/store.component.html +++ b/feature-libs/pickup-in-store/components/presentational/store/store.component.html @@ -7,6 +7,7 @@