Skip to content

Commit

Permalink
Use a queue instead of loops and timeouts
Browse files Browse the repository at this point in the history
  • Loading branch information
valentinpalkovic committed Dec 1, 2023
1 parent 3e338db commit 37e29e4
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 77 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { ICollection, StoryFnAngularReturnType } from '../types';
import { getApplication } from './StorybookModule';
import { storyPropsProvider } from './StorybookProvider';
import { PropertyExtractor } from './utils/PropertyExtractor';
import { bootstrapLock } from './utils/BootstrapLock';
import { queueBootstrapping } from './utils/BootstrapLock';

type StoryRenderInfo = {
storyFnAngular: StoryFnAngularReturnType;
Expand Down Expand Up @@ -128,7 +128,7 @@ export abstract class AbstractRenderer {
analyzedMetadata,
});

const applicationRef = await bootstrapLock(targetSelector, () => {
const applicationRef = await queueBootstrapping(() => {
return bootstrapApplication(application, {
...storyFnAngular.applicationConfig,
providers: [
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { Subject } from 'rxjs';

import { bootstrapLock, getCurrentLock } from './BootstrapLock';
import { queueBootstrapping } from './BootstrapLock';

const tick = (count: number) => new Promise((resolve) => setTimeout(resolve, count));
const tick = (count = 0) => new Promise((resolve) => setTimeout(resolve, count));

describe('BootstrapLock', () => {
beforeEach(async () => {
Expand All @@ -14,35 +14,28 @@ describe('BootstrapLock', () => {
});

it('should lock until complete', async () => {
expect(getCurrentLock()).toBeNull();

const pendingSubject = new Subject<void>();
const bootstrapApp = jest.fn().mockImplementation(async () => {
return pendingSubject.toPromise();
});
const bootstrapAppFinished = jest.fn();

bootstrapLock('story-1', bootstrapApp).then(bootstrapAppFinished);

await tick(30);
queueBootstrapping(bootstrapApp).then(() => {
bootstrapAppFinished();
});

expect(getCurrentLock()).toBe('story-1');
expect(bootstrapApp).toHaveBeenCalled();
expect(bootstrapAppFinished).not.toHaveBeenCalled();

pendingSubject.next();
pendingSubject.complete();

await tick(30);
await tick();

expect(bootstrapApp).toHaveReturnedTimes(1);
expect(bootstrapAppFinished).toHaveBeenCalled();
expect(getCurrentLock()).toBeNull();
});

it('should prevent second task, until first task complete', async () => {
expect(getCurrentLock()).toBeNull();

it('should prevent following tasks, until the preview tasks are complete', async () => {
const pendingSubject = new Subject<void>();
const bootstrapApp = jest.fn().mockImplementation(async () => {
return pendingSubject.toPromise();
Expand All @@ -55,64 +48,80 @@ describe('BootstrapLock', () => {
});
const bootstrapAppFinished2 = jest.fn();

bootstrapLock('story-1', bootstrapApp).then(bootstrapAppFinished);
bootstrapLock('story-2', bootstrapApp2).then(bootstrapAppFinished2);
const pendingSubject3 = new Subject<void>();
const bootstrapApp3 = jest.fn().mockImplementation(async () => {
return pendingSubject3.toPromise();
});
const bootstrapAppFinished3 = jest.fn();

queueBootstrapping(bootstrapApp).then(bootstrapAppFinished);
queueBootstrapping(bootstrapApp2).then(bootstrapAppFinished2);
queueBootstrapping(bootstrapApp3).then(bootstrapAppFinished3);

await tick(30);
await tick();

expect(getCurrentLock()).toBe('story-1');
expect(bootstrapApp).toHaveBeenCalled();
expect(bootstrapAppFinished).not.toHaveBeenCalled();
expect(bootstrapApp2).not.toHaveBeenCalled();
expect(bootstrapAppFinished2).not.toHaveBeenCalled();
expect(bootstrapApp3).not.toHaveBeenCalled();
expect(bootstrapAppFinished3).not.toHaveBeenCalled();

pendingSubject.next();
pendingSubject.complete();

await tick(30);
await tick();

expect(getCurrentLock()).toBe('story-2');
expect(bootstrapApp).toHaveReturnedTimes(1);
expect(bootstrapAppFinished).toHaveBeenCalled();
expect(bootstrapApp2).toHaveBeenCalled();
expect(bootstrapAppFinished2).not.toHaveBeenCalled();
expect(bootstrapApp3).not.toHaveBeenCalled();
expect(bootstrapAppFinished3).not.toHaveBeenCalled();

pendingSubject2.next();
pendingSubject2.complete();

await tick(30);
await tick();

expect(bootstrapApp).toHaveReturnedTimes(1);
expect(bootstrapAppFinished).toHaveBeenCalled();
expect(bootstrapApp2).toHaveReturnedTimes(1);
expect(bootstrapAppFinished2).toHaveBeenCalled();
expect(bootstrapApp3).toHaveBeenCalled();
expect(bootstrapAppFinished3).not.toHaveBeenCalled();

pendingSubject3.next();
pendingSubject3.complete();

await tick();

expect(getCurrentLock()).toBeNull();
expect(bootstrapApp).toHaveReturnedTimes(1);
expect(bootstrapAppFinished).toHaveBeenCalled();
expect(bootstrapApp2).toHaveReturnedTimes(1);
expect(bootstrapAppFinished2).toHaveBeenCalled();
expect(bootstrapApp3).toHaveReturnedTimes(1);
expect(bootstrapAppFinished3).toHaveBeenCalled();
});

it('should reset lock on error', async () => {
expect(getCurrentLock()).toBeNull();

const pendingSubject = new Subject<void>();
const bootstrapApp = jest.fn().mockImplementation(async () => {
return pendingSubject.toPromise();
});
const bootstrapAppFinished = jest.fn();
const bootstrapAppError = jest.fn();

bootstrapLock('story-1', bootstrapApp).then(bootstrapAppFinished).catch(bootstrapAppError);

await tick(30);
queueBootstrapping(bootstrapApp).then(bootstrapAppFinished).catch(bootstrapAppError);

expect(getCurrentLock()).toBe('story-1');
expect(bootstrapApp).toHaveBeenCalled();
expect(bootstrapAppFinished).not.toHaveBeenCalled();

// eslint-disable-next-line local-rules/no-uncategorized-errors
pendingSubject.error(new Error('test error'));

await tick(30);
await tick();

expect(getCurrentLock()).toBeNull();
expect(bootstrapApp).toHaveReturnedTimes(1);
expect(bootstrapAppFinished).not.toHaveBeenCalled();
expect(bootstrapAppError).toHaveBeenCalled();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,55 +1,28 @@
import { ApplicationRef } from '@angular/core';

let lock: string | null = null;
const queue: Array<() => Promise<void>> = [];
let isProcessing = false;

/**
* Returns the selector of the component being bootstrapped, or null if no
* component is being bootstrapped.
*/
export const getCurrentLock = (): string | null => {
return lock;
};
export const queueBootstrapping = (fn: () => Promise<ApplicationRef>): Promise<ApplicationRef> => {
return new Promise<ApplicationRef>((resolve, reject) => {
queue.push(() => fn().then(resolve).catch(reject));

/**
* Waits for chance to acquire lock for a component to bootstrap.
*
* @param selector the selectory of the component requesting a lock to bootstrap
* @returns
*/
const acquireLock = (selector: string) => {
return new Promise<void>((resolve) => {
function checkLock() {
if (lock === null) {
lock = selector;
resolve();
} else {
setTimeout(checkLock, 30);
}
if (!isProcessing) {
processQueue();
}

checkLock();
});
};

/**
* Delays bootstrapping until a lock is acquired, so that only one application
* can be bootstrapped at a time.
*
* Bootstrapping multiple applications at once can cause Angular to throw an
* error that a component is declared in multiple modules.
*
* @param selector the selectory of the component requesting a lock to bootstrap
* @param fn callback that should complete the bootstrap process
* @returns ApplicationRef from the completed bootstrap process
*/
export const bootstrapLock = async (selector: string, fn: () => Promise<ApplicationRef>) => {
await acquireLock(selector);
try {
const ref = await fn();
lock = null;
return ref;
} catch (e) {
lock = null;
throw e;
const processQueue = async () => {
isProcessing = true;

while (queue.length > 0) {
const bootstrappingFn = queue.shift();
if (bootstrappingFn) {
// eslint-disable-next-line no-await-in-loop
await bootstrappingFn();
}
}

isProcessing = false;
};

0 comments on commit 37e29e4

Please sign in to comment.