Skip to content

Commit a442740

Browse files
committed
feat(importer): detect duplicate destination filenames in generate.js
Two source folders writing SVGs with the same filename into the flat intermediate/ directory cause non-deterministic React atom output (the asynchronous fs.readdir + fs.stat + fs.copyFileSync race makes the final byte content depend on callback order). Add a Map-based detector that, at file copy time, records the first source folder to claim each destination. When a second source folder attempts to write the same destination, the collision is recorded and reported via a process.on('exit') hook after the async walker drains. Behaviour is controlled by a new --on-duplicate flag (fail|warn|ignore, default fail). A KNOWN_DUPLICATE_SOURCE regex allowlists the existing '<Name> Temp (LTR|RTL)/' folders so the build does not break today; those folders are staging artifacts that need a separate cleanup PR. Refs: #1009
1 parent f981da3 commit a442740

1 file changed

Lines changed: 87 additions & 2 deletions

File tree

importer/generate.js

Lines changed: 87 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,21 +42,29 @@ const parseArgs = (args=process.argv.slice(2)) => {
4242
describe: 'Keep original directories in output',
4343
default: false,
4444
})
45+
.option('on-duplicate', {
46+
type: 'string',
47+
choices: ['fail', 'warn', 'ignore'],
48+
describe: 'How to handle two source folders producing the same destination filename',
49+
default: 'fail',
50+
})
4551
.help()
4652
.wrap(Math.min(120, process.stdout.columns || 120))
4753
.argv;
4854

4955
/**
5056
* @typedef {'react'|'android'|'ios'|'flutter'} TargetType
5157
* @typedef {'svg'|'xml'|'pdf'|'tsx'} ExtensionType
58+
* @typedef {'fail'|'warn'|'ignore'} OnDuplicate
5259
*
5360
* @typedef {{
5461
* SRC_PATH: string,
5562
* DEST_PATH: string,
5663
* EXTENSION: ExtensionType,
5764
* TARGET: TargetType,
5865
* SELECTOR: boolean,
59-
* KEEP_DIRS: boolean
66+
* KEEP_DIRS: boolean,
67+
* ON_DUPLICATE: OnDuplicate,
6068
* }} ParseResult
6169
*/
6270

@@ -67,10 +75,30 @@ const parseArgs = (args=process.argv.slice(2)) => {
6775
TARGET: /** @type {TargetType} */ (argv.target),
6876
SELECTOR: Boolean(argv.selector),
6977
KEEP_DIRS: Boolean(argv.keepdirs),
78+
ON_DUPLICATE: /** @type {OnDuplicate} */ (argv['on-duplicate']),
7079
});
7180
};
7281

73-
const { SRC_PATH, DEST_PATH, EXTENSION, TARGET, SELECTOR, KEEP_DIRS } = parseArgs();
82+
const { SRC_PATH, DEST_PATH, EXTENSION, TARGET, SELECTOR, KEEP_DIRS, ON_DUPLICATE } = parseArgs();
83+
84+
/**
85+
* Source folders known to collide with canonical asset folders today.
86+
* These folders appear to be staging artifacts (no metadata.json, contain SVG
87+
* files named after the OPPOSITE icon, e.g. `Arrow Undo Temp RTL/SVG/ic_fluent_arrow_redo_*.svg`).
88+
* They cause non-deterministic atom output because multiple source folders
89+
* write to the same intermediate filename — see issue tracker for cleanup.
90+
* Until they are removed from `assets/`, collisions involving these folders
91+
* are downgraded to warnings so the build does not fail today.
92+
*
93+
* TODO: remove this allowlist once the `* Temp (LTR|RTL)` folders are deleted
94+
* from `assets/`.
95+
*/
96+
const KNOWN_DUPLICATE_SOURCE = / Temp (LTR|RTL)(\/|\\|$)/;
97+
98+
/** @type {Map<string, string>} destFile -> first srcFile that wrote it */
99+
const writtenFiles = new Map();
100+
/** @type {Array<{ destFile: string, previousSrc: string, conflictingSrc: string, allowlisted: boolean }>} */
101+
const duplicateCollisions = [];
74102
const ICON_OUTLINE_STYLE = '_regular'
75103
const ICON_FILLED_STYLE = '_filled'
76104
const ICON_LIGHT_STYLE = '_light'
@@ -166,6 +194,14 @@ function processFolder(srcPath, destPath, folderDepth) {
166194
if (!fs.existsSync(destPath)) {
167195
fs.mkdirSync(destPath)
168196
}
197+
const previousSrc = writtenFiles.get(destFile);
198+
if (previousSrc && previousSrc !== srcFile) {
199+
const allowlisted =
200+
KNOWN_DUPLICATE_SOURCE.test(previousSrc) || KNOWN_DUPLICATE_SOURCE.test(srcFile);
201+
duplicateCollisions.push({ destFile, previousSrc, conflictingSrc: srcFile, allowlisted });
202+
} else {
203+
writtenFiles.set(destFile, srcFile);
204+
}
169205
fs.copyFileSync(srcFile, destFile);
170206
// Generate selector if both filled/regular styles are available
171207
if (SELECTOR && file.endsWith(SVG_EXTENSION)) {
@@ -240,3 +276,52 @@ export default ${iconName + REACT_SUFFIX};
240276
if (err) throw err;
241277
});
242278
}
279+
280+
/**
281+
* Report any duplicate-destination collisions detected during the walk.
282+
*
283+
* The walker is async-callback based and has no explicit completion signal,
284+
* so we hook `process.on('exit')` which fires after all queued callbacks drain
285+
* and Node would otherwise exit naturally.
286+
*
287+
* Behaviour per `--on-duplicate`:
288+
* - fail (default): non-zero exit if any non-allowlisted collision occurred
289+
* - warn : prints all collisions but keeps exit code 0
290+
* - ignore : prints nothing
291+
*
292+
* Collisions where either side matches `KNOWN_DUPLICATE_SOURCE` are downgraded
293+
* to a warning even under `--on-duplicate=fail`.
294+
*/
295+
process.on('exit', () => {
296+
if (ON_DUPLICATE === 'ignore' || duplicateCollisions.length === 0) {
297+
return;
298+
}
299+
const blocking = duplicateCollisions.filter((c) => !c.allowlisted);
300+
const allowlisted = duplicateCollisions.filter((c) => c.allowlisted);
301+
/** @param {typeof duplicateCollisions} list */
302+
const formatList = (list) =>
303+
list
304+
.map(
305+
(c) =>
306+
` - ${c.destFile}\n first : ${c.previousSrc}\n collide: ${c.conflictingSrc}`
307+
)
308+
.join('\n');
309+
if (allowlisted.length > 0) {
310+
console.warn(
311+
`\n[generate.js] ${allowlisted.length} known duplicate destination(s) ` +
312+
`(allowlisted via KNOWN_DUPLICATE_SOURCE):\n${formatList(allowlisted)}\n`
313+
);
314+
}
315+
if (blocking.length > 0) {
316+
const message =
317+
`\n[generate.js] ${blocking.length} duplicate destination(s) detected. ` +
318+
`Two source folders produced the same output filename; output is non-deterministic. ` +
319+
`Rename one of the source folders or remove the duplicate asset.\n${formatList(blocking)}\n`;
320+
if (ON_DUPLICATE === 'fail') {
321+
console.error(message);
322+
process.exitCode = 1;
323+
} else {
324+
console.warn(message);
325+
}
326+
}
327+
});

0 commit comments

Comments
 (0)