Skip to content

Commit

Permalink
Fix synthesized template-only components crash
Browse files Browse the repository at this point in the history
This fixes the first scenario detailed in #1619, where a synthesized
template-only component .js file is not cleaned up when the corresponding
.hbs file is deleted.

There are actually two bugs here.

The first issue is we never attempt to clean up these unneeded files,
which is fixed by keeping track of which of the emitted files are still
needed after the main loop.

The second issue is that, even with that fix, it still does not work,
because we are incorrectly using the async version of the fs API here
without awaiting for the promise.

With the way the promisify code is written, the fs delete is actually
scheduled to happen on the microtask queue, so by the time the
`mergeTrees` code runs, the deletion hasn't actually happened yet.

Since the synthesized tree appears after the appTree, I believe (but
haven't tested) this has also been causing a different bug, where if you
started with just a `.hbs` file, then add a `.js` file later, the new file
does not actually take effect until one more build later, since the build
triggered by the `.js` file creation would get shadowed by the previously
synthesized template-only component file.

Perhaps no caught this issue because the typical workflow involves
creating either an empty file or generating a stub `.js` file, then filling
it in with the actual content. By the "first meaningful save", the deletion
would probably have gone through and this would self-resolve.
  • Loading branch information
chancancode committed Oct 3, 2023
1 parent 1e88426 commit a390c25
Showing 1 changed file with 7 additions and 2 deletions.
9 changes: 7 additions & 2 deletions packages/compat/src/synthesize-template-only-components.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import Plugin from 'broccoli-plugin';
import type { Node } from 'broccoli-node-api';
import { join, basename } from 'path';
import walkSync from 'walk-sync';
import { remove, outputFileSync, pathExistsSync } from 'fs-extra';
import { removeSync, outputFileSync, pathExistsSync } from 'fs-extra';

const source = `import templateOnlyComponent from '@ember/component/template-only';
export default templateOnlyComponent();`;
Expand All @@ -25,17 +25,22 @@ export default class SynthesizeTemplateOnlyComponents extends Plugin {
}

async build() {
let unneeded = new Set(this.emitted);
for (let dir of this.allowedPaths) {
let { needed, seen } = this.crawl(join(this.inputPaths[0], dir));
for (let file of needed) {
let fullName = join(this.outputPath, dir, file);
unneeded.delete(fullName);
if (seen.has(file)) {
this.remove(fullName);
} else {
this.add(fullName);
}
}
}
for (let fullName of unneeded) {
this.remove(fullName);
}
}
private add(filename: string) {
if (!this.emitted.has(filename)) {
Expand All @@ -51,7 +56,7 @@ export default class SynthesizeTemplateOnlyComponents extends Plugin {

private remove(filename: string) {
if (this.emitted.has(filename)) {
remove(filename + '.js');
removeSync(filename + '.js');
this.emitted.delete(filename);
}
}
Expand Down

0 comments on commit a390c25

Please sign in to comment.