Skip to content

Commit 61d0e54

Browse files
committed
fix: Avoid Promise.all() because of possible race conditions.
1 parent afd2f14 commit 61d0e54

File tree

7 files changed

+38
-7
lines changed

7 files changed

+38
-7
lines changed

packages/@css-blocks/core/src/Analyzer/Analysis.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
import { ObjectDictionary, objectValues } from "@opticss/util";
1515
import { IdentGenerator } from "opticss";
1616

17+
import { allDone } from "../util";
1718
import { BlockFactory } from "../BlockParser";
1819
import { DEFAULT_EXPORT } from "../BlockSyntax";
1920
import { Block, Style } from "../BlockTree";
@@ -354,7 +355,7 @@ export class Analysis<K extends keyof TemplateTypes> {
354355
});
355356
blockPromises.push(promise);
356357
});
357-
let values = await Promise.all(blockPromises);
358+
let values = await allDone(blockPromises);
358359

359360
// Create a temporary block so we can take advantage of `Block.lookup`
360361
// to easily resolve all BlockPaths referenced in the serialized analysis.

packages/@css-blocks/core/src/BlockParser/features/export-blocks.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { postcss } from "opticss";
33
import { BLOCK_EXPORT, CLASS_NAME_IDENT, DEFAULT_EXPORT, RESERVED_BLOCK_NAMES } from "../../BlockSyntax";
44
import { Block } from "../../BlockTree";
55
import * as errors from "../../errors";
6+
import { allDone } from "../../util";
67
import { sourceRange } from "../../SourceLocation";
78
import { BlockFactory } from "../index";
89
import { parseBlockNames, stripQuotes } from "../utils";
@@ -114,6 +115,6 @@ export async function exportBlocks(block: Block, factory: BlockFactory, file: st
114115
}
115116

116117
// After all export promises have resolved, resolve the decorated Block.
117-
await Promise.all(exportPromises);
118+
await allDone(exportPromises);
118119
return block;
119120
}

packages/@css-blocks/core/src/BlockParser/features/import-blocks.ts

+3-2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { BLOCK_IMPORT, CLASS_NAME_IDENT, DEFAULT_EXPORT, isBlockNameReserved } f
55
import { Block } from "../../BlockTree";
66
import * as errors from "../../errors";
77
import { sourceRange } from "../../SourceLocation";
8+
import { allDone } from "../../util";
89
import { BlockFactory } from "../index";
910
import { parseBlockNames, stripQuotes } from "../utils";
1011

@@ -96,9 +97,9 @@ export async function importBlocks(block: Block, factory: BlockFactory, file: st
9697

9798
// When all import promises have resolved, save the block references and
9899
// resolve.
99-
let results;
100+
let results: Array<[string, string, postcss.AtRule, Block]>;
100101
try {
101-
results = await Promise.all(namedBlockReferences);
102+
results = await allDone(namedBlockReferences);
102103
let localNames: ObjectDictionary<string> = {};
103104
results.forEach(([localName, importPath, atRule, otherBlock]) => {
104105
if (localNames[localName]) {

packages/@css-blocks/core/src/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,4 @@ export * from "./importing";
1515
export * from "./Analyzer";
1616
export * from "./TemplateRewriter";
1717
export * from "./configuration";
18+
export * from "./util";
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/**
2+
* This behaves like Promise.all() but it doesn't resolve or reject until
3+
* all the work is done.
4+
*
5+
* Note: if multiple promises reject, the error returned is based on the order
6+
* of the promises passed in (the reason from promise with the lowest index is
7+
* returned).
8+
* @template T What each promise returns.
9+
* @param promises The promises to wait for.
10+
* @returns An array of results or rejects with the first error that it encounters.
11+
*/
12+
export async function allDone<T>(promises: Array<Promise<T>>): Promise<Array<T>> {
13+
let results = new Array<T>();
14+
let firstError = null;
15+
for (let p of promises) {
16+
try {
17+
results.push(await p);
18+
} catch (e) {
19+
if (!firstError) firstError = e;
20+
}
21+
}
22+
if (firstError) {
23+
throw firstError;
24+
} else {
25+
return results;
26+
}
27+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
export { allDone } from "./allDone";
2+
export { unionInto } from "./unionInto";

packages/@css-blocks/glimmer/src/Analyzer.ts

+1-3
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@ export class GlimmerAnalyzer extends Analyzer<TEMPLATE_TYPE> {
6161

6262
async analyze(dir: string, componentNames: string[]): Promise<GlimmerAnalyzer> {
6363
let components = new Set<string>(componentNames);
64-
let analysisPromises: Promise<GlimmerAnalysis>[] = [];
6564
this.debug(`Analyzing all templates starting with: ${componentNames}`);
6665

6766
for (let component of components) {
@@ -73,10 +72,9 @@ export class GlimmerAnalyzer extends Analyzer<TEMPLATE_TYPE> {
7372
this.debug(`Analyzing all components: ${[...components].join(", ")}`);
7473

7574
for (let component of components) {
76-
analysisPromises.push(this.analyzeTemplate(dir, component));
75+
await this.analyzeTemplate(dir, component);
7776
}
7877

79-
await Promise.all(analysisPromises);
8078
return this;
8179
}
8280

0 commit comments

Comments
 (0)