diff --git a/packages/compat/src/synthesize-template-only-components.ts b/packages/compat/src/synthesize-template-only-components.ts index 62020455e..dee331730 100644 --- a/packages/compat/src/synthesize-template-only-components.ts +++ b/packages/compat/src/synthesize-template-only-components.ts @@ -1,16 +1,29 @@ 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 { join, basename, extname } from 'path'; +import walkSync, { type Entry } from 'walk-sync'; +import { removeSync, outputFileSync, pathExistsSync, readFileSync } from 'fs-extra'; const source = `import templateOnlyComponent from '@ember/component/template-only'; export default templateOnlyComponent();`; const jsExtensions = ['.js', '.ts', '.mjs', '.mts']; +type Emitted = + | { type: 'template-only-component'; outputPath: string } + | { type: 'template-import'; outputPath: string; mtime: number }; + +type TemplateOnly = { template: Entry; javascript: undefined }; +type JavaScriptOnly = { template: undefined; javascript: Entry }; +type Colocated = { template: Entry; javascript: Entry }; +type ComponentFiles = TemplateOnly | JavaScriptOnly | Colocated; + +function importTemplate(files: { template: Entry }): string { + return `/* import __COLOCATED_TEMPLATE__ from './${basename(files.template.relativePath)}'; */\n`; +} + export default class SynthesizeTemplateOnlyComponents extends Plugin { - private emitted = new Set() as Set; + private emitted = new Map() as Map; private allowedPaths: string[]; private templateExtensions: string[]; @@ -25,54 +38,92 @@ export default class SynthesizeTemplateOnlyComponents extends Plugin { } async build() { + let unneeded = new Set(this.emitted.keys()); 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); - if (seen.has(file)) { - this.remove(fullName); + let entries = this.crawl(join(this.inputPaths[0], dir)); + for (let [name, files] of entries) { + let fullName = join(this.outputPath, dir, name); + unneeded.delete(fullName); + if (files.javascript && files.template) { + this.addTemplateImport(fullName, files); + } else if (files.template) { + this.addTemplateOnlyComponent(fullName, files); } else { - this.add(fullName); + this.remove(fullName); } } } + for (let fullName of unneeded) { + this.remove(fullName); + } } - private add(filename: string) { - if (!this.emitted.has(filename)) { + + private addTemplateOnlyComponent(filename: string, files: TemplateOnly) { + const emitted = this.emitted.get(filename); + + if (emitted?.type !== 'template-only-component') { // special case: ember-cli doesn't allow template-only components named // "template.hbs" because there are too many people doing a "pods-like" // layout that happens to match that pattern.🤮 if (basename(filename) !== 'template') { - outputFileSync(filename + '.js', source, 'utf8'); + const outputPath = filename + '.js'; + outputFileSync(outputPath, importTemplate(files) + source, 'utf8'); + this.emitted.set(filename, { type: 'template-only-component', outputPath }); + + if (emitted && emitted.outputPath !== outputPath) { + removeSync(emitted.outputPath); + } + } + } + } + + private addTemplateImport(filename: string, files: Colocated) { + const emitted = this.emitted.get(filename); + const mtime = files.javascript.mtime; + + if (!(emitted?.type === 'template-import' && emitted.mtime === mtime)) { + const inputSource = readFileSync(files.javascript.fullPath, { encoding: 'utf8' }); + const outputPath = filename + extname(files.javascript.relativePath); + // If we are ok with appending instead, copy + append maybe more efficient? + outputFileSync(outputPath, importTemplate(files) + inputSource, 'utf8'); + this.emitted.set(filename, { type: 'template-import', outputPath, mtime }); + + if (emitted && emitted.outputPath !== outputPath) { + removeSync(emitted.outputPath); } - this.emitted.add(filename); } } private remove(filename: string) { - if (this.emitted.has(filename)) { - remove(filename + '.js'); + const emitted = this.emitted.get(filename); + + if (emitted) { + removeSync(emitted.outputPath); this.emitted.delete(filename); } } - private crawl(dir: string) { - const needed = new Set(); - const seen = new Set(); + private crawl(dir: string): Map { + const entries = new Map(); + if (pathExistsSync(dir)) { - for (let file of walkSync(dir, { directories: false })) { - for (const templateExtension of this.templateExtensions) { - if (file.endsWith(templateExtension)) { - needed.add(file.slice(0, -1 * templateExtension.length)); - } else { - const jsExtension = jsExtensions.find(ext => file.endsWith(ext)); - if (jsExtension) { - seen.add(file.slice(0, -1 * jsExtension.length)); - } - } + for (let entry of walkSync.entries(dir, { directories: false })) { + const templateExtension = this.templateExtensions.find(ext => entry.relativePath.endsWith(ext)); + if (templateExtension) { + const key = entry.relativePath.slice(0, -1 * templateExtension.length); + entries.set(key, { template: entry, javascript: entries.get(key)?.javascript }); + continue; + } + + const jsExtension = jsExtensions.find(ext => entry.relativePath.endsWith(ext)); + if (jsExtension) { + const key = entry.relativePath.slice(0, -1 * jsExtension.length); + entries.set(key, { template: entries.get(key)?.template, javascript: entry }); + continue; } } } - return { needed, seen }; + + return entries; } } diff --git a/tests/scenarios/watch-mode-test.ts b/tests/scenarios/watch-mode-test.ts index 3bf8a6945..8d87d5fbe 100644 --- a/tests/scenarios/watch-mode-test.ts +++ b/tests/scenarios/watch-mode-test.ts @@ -3,6 +3,7 @@ import type { PreparedApp } from 'scenario-tester'; import QUnit from 'qunit'; import globby from 'globby'; import fs from 'fs/promises'; +import { pathExists } from 'fs-extra'; import path from 'path'; import execa, { type Options, type ExecaChildProcess } from 'execa'; @@ -106,17 +107,16 @@ class OutputWaiter extends Waiter { } } -type Status = { type: 'starting' } | { type: 'ready' } | { type: 'errored'; error: unknown } | { type: 'completed' }; +type Status = { type: 'running' } | { type: 'errored'; error: unknown } | { type: 'completed' }; class EmberCLI { - static launch(args: readonly string[], options: Options): EmberCLI { + static launch(args: readonly string[], options: Options = {}): EmberCLI { return new EmberCLI(execa('ember', args, { ...options, all: true })); } - readonly ready: Promise; readonly completed: Promise; - private status: Status = { type: 'starting' }; + private status: Status = { type: 'running' }; private waiters: Waiter[] = []; private lines: string[] = []; @@ -137,14 +137,6 @@ class EmberCLI { this.waiters = []; }); - const ready = new OutputWaiter(this, /Serving on http:\/\/localhost:[0-9]+\//, DEFAULT_TIMEOUT * 2); - - this.waiters.push(ready); - - this.ready = ready.promise.then(() => { - this.status = { type: 'ready' }; - }); - const exit = new (class ExitWaiter extends Waiter { constructor(private process: EmberCLI) { super(null); @@ -183,12 +175,8 @@ class EmberCLI { }); } - get isStarting(): boolean { - return this.status.type === 'starting'; - } - - get isReady(): boolean { - return this.status.type === 'ready'; + get isRunning(): boolean { + return this.status.type === 'running'; } get isErrored(): boolean { @@ -239,13 +227,153 @@ class EmberCLI { } } +class File { + constructor(readonly label: string, readonly fullPath: string) {} + + async exists(): Promise { + return pathExists(this.fullPath); + } + + async read(): Promise { + try { + return await fs.readFile(this.fullPath, { encoding: 'utf-8' }); + } catch (error) { + if (error.code === 'ENOENT') { + return null; + } else { + throw error; + } + } + } + + async write(content: string): Promise { + await fs.writeFile(this.fullPath, content, { encoding: 'utf-8' }); + } + + async delete(): Promise { + await fs.unlink(this.fullPath); + } +} + +class AssertFile { + readonly file: File; + + constructor(private assert: Assert, file: File) { + this.file = file; + } + + async exists(): Promise { + this.assert.true(await this.file.exists(), `${this.file.label} exists`); + } + + async doesNotExist(): Promise { + this.assert.false(await this.file.exists(), `${this.file.label} does not exists`); + } + + async hasContent(expected: string): Promise { + let actual = await this.file.read(); + + if (actual === null) { + this.assert.ok(false, `${this.file.label} does not exists`); + } else { + this.assert.equal(actual, expected, `content of ${this.file.label}`); + } + } + + async doesNotHaveContent(expected: string | RegExp): Promise { + let actual = await this.file.read(); + + if (actual === null) { + this.assert.ok(false, `${this.file.label} does not exists`); + } else { + this.assert.notEqual(actual, expected, `content of ${this.file.label}`); + } + } + + async includesContent(expected: string): Promise { + let actual = await this.file.read(); + + if (actual === null) { + this.assert.ok(false, `${this.file.label} does not exists`); + } else { + this.assert.true(actual.includes(expected), `content of ${this.file.label}`); + } + } + + async doesNotIncludeContent(expected: string): Promise { + let actual = await this.file.read(); + + if (actual === null) { + this.assert.ok(false, `${this.file.label} does not exists`); + } else { + this.assert.false(actual.includes(expected), `content of ${this.file.label}`); + } + } +} + +function d(strings: TemplateStringsArray, ...values: unknown[]): string { + let buf = ''; + for (let string of strings) { + if (values.length) { + buf += string + values.shift(); + } else { + buf += string; + } + } + return deindent(buf); +} + +function deindent(s: string): string { + if (s.startsWith('\n')) { + s = s.slice(1); + } + + let indentSize = s.search(/\S/); + + if (indentSize > 0) { + let indent = s.slice(0, indentSize); + + s = s + .split('\n') + .map(line => { + if (line.startsWith(indent)) { + return line.slice(indentSize); + } else { + return line; + } + }) + .join('\n'); + } + + s = s.trimEnd(); + + return s; +} + app.forEachScenario(scenario => { Qmodule(scenario.name, function (hooks) { let app: PreparedApp; - let cli: EmberCLI; + let server: EmberCLI; + + function appFile(appPath: string): File { + let fullPath = path.join(app.dir, ...appPath.split('/')); + return new File(appPath, fullPath); + } async function waitFor(...args: Parameters): Promise { - await cli.waitFor(...args); + await server.waitFor(...args); + } + + async function added(filePath: string): Promise { + await waitFor(`file added ${path.join(...filePath.split('/'))}`); + } + + async function changed(filePath: string): Promise { + await waitFor(`file changed ${path.join(...filePath.split('/'))}`); + } + + async function deleted(filePath: string): Promise { + await waitFor(`file deleted ${path.join(...filePath.split('/'))}`); } async function checkScripts(distPattern: RegExp, needle: string) { @@ -263,13 +391,13 @@ app.forEachScenario(scenario => { hooks.beforeEach(async () => { app = await scenario.prepare(); - cli = EmberCLI.launch(['serve', '--port', '0'], { cwd: app.dir }); - await cli.ready; - cli.clearOutput(); + server = EmberCLI.launch(['serve', '--port', '0'], { cwd: app.dir }); + await waitFor(/Serving on http:\/\/localhost:[0-9]+\//, DEFAULT_TIMEOUT * 2); + server.clearOutput(); }); hooks.afterEach(async () => { - await cli.shutdown(); + await server.shutdown(); }); test(`ember serve`, async function (assert) { @@ -277,23 +405,210 @@ app.forEachScenario(scenario => { 'TWO IS A GREAT NUMBER< I LKE IT A LOT< IT IS THE POWER OF ALL OF ELECTRONICS, MATH, ETC'; assert.false(await checkScripts(/js$/, originalContent), 'file has not been created yet'); - await fs.writeFile(path.join(app.dir, 'app/simple-file.js'), `export const two = "${originalContent}";`); - await waitFor('file added simple-file.js'); + await appFile('app/simple-file.js').write(`export const two = "${originalContent}";`); + await added('simple-file.js'); await waitFor(/Build successful/); assert.true(await checkScripts(/js$/, originalContent), 'the file now exists'); - cli.clearOutput(); + server.clearOutput(); const updatedContent = 'THREE IS A GREAT NUMBER TWO'; assert.false(await checkScripts(/js$/, updatedContent), 'file has not been created yet'); - await fs.writeFile(path.join(app.dir, 'app/simple-file.js'), `export const two = "${updatedContent}";`); - await waitFor('file changed simple-file.js'); + await appFile('app/simple-file.js').write(`export const two = "${updatedContent}";`); + await changed('simple-file.js'); await waitFor(/Build successful/); // TODO: find a better way to test this; this seems to linger around // assert.false(await checkScripts(/js$/, originalContent), 'the original file does not exists'); assert.true(await checkScripts(/js$/, updatedContent), 'the updated file now exists'); }); + + Qmodule('[GH#1619] co-located components regressions', function (hooks) { + // These tests uses the internal `.rewritten-app` structure to confirm the failures. + // If that changes these tests should be updated to match the spirit of the original + // issue (https://github.com/embroider-build/embroider/issues/1619) + let assertRewrittenFile: (rewrittenPath: string) => AssertFile; + + hooks.beforeEach(assert => { + assertRewrittenFile = (rewrittenPath: string) => { + let fullPath = path.join(app.dir, 'node_modules', '.embroider', 'rewritten-app', ...rewrittenPath.split('/')); + let file = new File(rewrittenPath, fullPath); + return new AssertFile(assert, file); + }; + }); + + test('Scenario 1: deleting a template-only component', async function () { + await assertRewrittenFile('assets/app-template.js').doesNotIncludeContent( + '"app-template/components/hello-world"' + ); + await assertRewrittenFile('components/hello-world.hbs').doesNotExist(); + await assertRewrittenFile('components/hello-world.js').doesNotExist(); + + await appFile('app/components/hello-world.hbs').write('hello world!'); + await added('components/hello-world.hbs'); + await waitFor(/Build successful/); + await assertRewrittenFile('assets/app-template.js').includesContent('"app-template/components/hello-world"'); + await assertRewrittenFile('components/hello-world.hbs').hasContent('hello world!'); + await assertRewrittenFile('components/hello-world.js').hasContent(d` + /* import __COLOCATED_TEMPLATE__ from './hello-world.hbs'; */ + import templateOnlyComponent from '@ember/component/template-only'; + export default templateOnlyComponent(); + `); + server.clearOutput(); + + await appFile('app/components/hello-world.hbs').delete(); + await deleted('components/hello-world.hbs'); + await waitFor(/Build successful/); + await assertRewrittenFile('assets/app-template.js').doesNotIncludeContent( + '"app-template/components/hello-world"' + ); + await assertRewrittenFile('components/hello-world.hbs').doesNotExist(); + await assertRewrittenFile('components/hello-world.js').doesNotExist(); + }); + + test('Scenario 2: adding a template to a component', async function (assert) { + await assertRewrittenFile('components/hello-world.hbs').doesNotExist(); + await assertRewrittenFile('components/hello-world.js').doesNotExist(); + await assertRewrittenFile('tests/integration/hello-world-test.js').doesNotExist(); + + await appFile('tests/integration/hello-world-test.js').write(d` + import { module, test } from 'qunit'; + import { setupRenderingTest } from 'ember-qunit'; + import { render } from '@ember/test-helpers'; + import { hbs } from 'ember-cli-htmlbars'; + + module('Integration | hello-world', function(hooks) { + setupRenderingTest(hooks); + + test('it renders', async function(assert) { + await render(hbs\`\`); + assert.dom(this.element).hasText('hello world!'); + }); + }); + `); + await added('integration/hello-world-test.js'); + await waitFor(/Build successful/); + await assertRewrittenFile('components/hello-world.hbs').doesNotExist(); + await assertRewrittenFile('components/hello-world.js').doesNotExist(); + await assertRewrittenFile('tests/integration/hello-world-test.js').includesContent(''); + server.clearOutput(); + + await appFile('app/components/hello-world.js').write(d` + import Component from '@glimmer/component'; + export default class extends Component {} + `); + await added('components/hello-world.js'); + await waitFor(/Build successful/); + await assertRewrittenFile('components/hello-world.hbs').doesNotExist(); + await assertRewrittenFile('components/hello-world.js').hasContent(d` + import Component from '@glimmer/component'; + export default class extends Component {} + `); + await assertRewrittenFile('tests/integration/hello-world-test.js').includesContent(''); + server.clearOutput(); + + let test = await EmberCLI.launch(['test', '--filter', 'hello-world'], { cwd: app.dir }); + await test.waitFor(/^not ok .+ Integration | hello-world: it renders/, DEFAULT_TIMEOUT * 2); + await assert.rejects(test.completed); + + await appFile('app/components/hello-world.hbs').write('hello world!'); + await added('components/hello-world.hbs'); + await waitFor(/Build successful/); + await assertRewrittenFile('components/hello-world.hbs').hasContent('hello world!'); + await assertRewrittenFile('components/hello-world.js').hasContent(d` + /* import __COLOCATED_TEMPLATE__ from './hello-world.hbs'; */ + import Component from '@glimmer/component'; + export default class extends Component {} + `); + await assertRewrittenFile('tests/integration/hello-world-test.js').includesContent(''); + + test = await EmberCLI.launch(['test', '--filter', 'hello-world'], { cwd: app.dir }); + await test.waitFor(/^ok .+ Integration | hello-world: it renders/); + await test.completed; + }); + + test('Scenario 3: deleting a co-located template', async function () { + await assertRewrittenFile('components/hello-world.hbs').doesNotExist(); + await assertRewrittenFile('components/hello-world.js').doesNotExist(); + + await appFile('app/components/hello-world.hbs').write('hello world!'); + await added('components/hello-world.hbs'); + await waitFor(/Build successful/); + await assertRewrittenFile('components/hello-world.hbs').hasContent('hello world!'); + await assertRewrittenFile('components/hello-world.js').hasContent(d` + /* import __COLOCATED_TEMPLATE__ from './hello-world.hbs'; */ + import templateOnlyComponent from '@ember/component/template-only'; + export default templateOnlyComponent(); + `); + server.clearOutput(); + + await appFile('app/components/hello-world.js').write(d` + import Component from '@glimmer/component'; + export default class extends Component {} + `); + await added('components/hello-world.js'); + await waitFor(/Build successful/); + await assertRewrittenFile('components/hello-world.hbs').hasContent('hello world!'); + await assertRewrittenFile('components/hello-world.js').hasContent(d` + /* import __COLOCATED_TEMPLATE__ from './hello-world.hbs'; */ + import Component from '@glimmer/component'; + export default class extends Component {} + `); + server.clearOutput(); + + await appFile('app/components/hello-world.hbs').delete(); + await deleted('components/hello-world.hbs'); + await waitFor(/Build successful/); + await assertRewrittenFile('components/hello-world.hbs').doesNotExist(); + await assertRewrittenFile('components/hello-world.js').hasContent(d` + import Component from '@glimmer/component'; + export default class extends Component {} + `); + }); + + test('Scenario 4: editing a co-located js file', async function () { + await assertRewrittenFile('components/hello-world.hbs').doesNotExist(); + await assertRewrittenFile('components/hello-world.js').doesNotExist(); + + await appFile('app/components/hello-world.hbs').write('hello world!'); + await added('components/hello-world.hbs'); + await waitFor(/Build successful/); + server.clearOutput(); + + await appFile('app/components/hello-world.js').write(d` + import Component from '@glimmer/component'; + export default class extends Component {} + `); + await added('components/hello-world.js'); + await waitFor(/Build successful/); + server.clearOutput(); + + await assertRewrittenFile('components/hello-world.hbs').hasContent('hello world!'); + await assertRewrittenFile('components/hello-world.js').hasContent(d` + /* import __COLOCATED_TEMPLATE__ from './hello-world.hbs'; */ + import Component from '@glimmer/component'; + export default class extends Component {} + `); + + await appFile('app/components/hello-world.js').write(d` + import Component from '@glimmer/component'; + export default class extends Component { + // this shows that updates invalidate any caches and reflects properly + } + `); + await changed('components/hello-world.js'); + await waitFor(/Build successful/); + + await assertRewrittenFile('components/hello-world.hbs').hasContent('hello world!'); + await assertRewrittenFile('components/hello-world.js').hasContent(d` + /* import __COLOCATED_TEMPLATE__ from './hello-world.hbs'; */ + import Component from '@glimmer/component'; + export default class extends Component { + // this shows that updates invalidate any caches and reflects properly + } + `); + }); + }); }); });