diff --git a/lib/models/builder.js b/lib/models/builder.js index 12e7562030..e016467055 100644 --- a/lib/models/builder.js +++ b/lib/models/builder.js @@ -26,7 +26,6 @@ class Builder extends CoreObject { // Use Broccoli 2.0 by default, if this fails due to .read/.rebuild API, fallback to broccoli-builder this.broccoliBuilderFallback = false; - this.setupBroccoliBuilder(); this._instantiationStack = new Error().stack.replace(/[^\n]*\n/, ''); this._cleanup = this.cleanup.bind(this); @@ -42,25 +41,31 @@ class Builder extends CoreObject { * @method readBuildFile * @param path The file path to read the build file from */ - readBuildFile(path) { + async readBuildFile(path) { // Load the build file - let buildFile = findBuildFile('ember-cli-build.js', path); + let buildFile = await findBuildFile(path); if (buildFile) { - return buildFile({ project: this.project }); + return await buildFile({ project: this.project }); } throw new SilentError('No ember-cli-build.js found.'); } + async ensureBroccoliBuilder() { + if (this.builder === undefined) { + await this.setupBroccoliBuilder(); + } + } + /** * @private * @method setupBroccoliBuilder */ - setupBroccoliBuilder() { + async setupBroccoliBuilder() { this.environment = this.environment || 'development'; process.env.EMBER_ENV = process.env.EMBER_ENV || this.environment; - this.tree = this.readBuildFile(this.project.root); + this.tree = await this.readBuildFile(this.project.root); let broccoli = require('broccoli'); @@ -167,6 +172,8 @@ class Builder extends CoreObject { * @return {Promise} */ async build(addWatchDirCallback, resultAnnotation) { + await this.ensureBroccoliBuilder(); + let buildResults, uiProgressIntervalID; try { diff --git a/lib/models/server-watcher.js b/lib/models/server-watcher.js index fe53c2e945..d2e4677e63 100644 --- a/lib/models/server-watcher.js +++ b/lib/models/server-watcher.js @@ -3,8 +3,8 @@ const Watcher = require('./watcher'); module.exports = class ServerWatcher extends Watcher { - constructor(options) { - super(options); + constructor(options, build) { + super(options, build); this.watcher.on('add', this.didAdd.bind(this)); this.watcher.on('delete', this.didDelete.bind(this)); diff --git a/lib/models/watcher.js b/lib/models/watcher.js index 380867687b..4177f8c6d7 100644 --- a/lib/models/watcher.js +++ b/lib/models/watcher.js @@ -12,19 +12,35 @@ const eventTypeNormalization = { change: 'changed', }; +const ConstructedFromBuilder = Symbol('Watcher.build'); + module.exports = class Watcher extends CoreObject { - constructor(_options) { + constructor(_options, build) { + if (build !== ConstructedFromBuilder) { + throw new Error('instantiate Watcher with (await Watcher.build()).watcher, not new Watcher()'); + } + super(_options); this.verbose = true; + this.serving = _options.serving; + } - let options = this.buildOptions(); + static async build(_options) { + let watcher = new this(_options, ConstructedFromBuilder); + await watcher.setupBroccoliWatcher(_options); - logger.info('initialize %o', options); + // This indirection is because Watcher instances are themselves spec + // noncompliant thennables (see the then() method) so returning watcher + // directly will interfere with `await Watcher.build()` + return { watcher }; + } - this.serving = _options.serving; + async setupBroccoliWatcher() { + let options = this.buildOptions(); - this.watcher = this.watcher || this.constructBroccoliWatcher(options); + logger.info('initialize %o', options); + this.watcher = this.watcher || (await this.constructBroccoliWatcher(options)); this.setupBroccoliChangeEvent(); this.watcher.on('buildStart', this._setupBroccoliWatcherBuild.bind(this)); @@ -38,8 +54,9 @@ module.exports = class Watcher extends CoreObject { this.serveURL = serveURL; } - constructBroccoliWatcher(options) { + async constructBroccoliWatcher(options) { const { Watcher } = require('broccoli'); + await this.builder.ensureBroccoliBuilder(); const { watchedSourceNodeWrappers } = this.builder.builder; let watcher = new Watcher(this.builder, watchedSourceNodeWrappers, { saneOptions: options, ignored: this.ignored }); diff --git a/lib/tasks/build-watch.js b/lib/tasks/build-watch.js index c390088981..e73bce9f5d 100644 --- a/lib/tasks/build-watch.js +++ b/lib/tasks/build-watch.js @@ -35,13 +35,15 @@ class BuildWatchTask extends Task { let watcher = options._watcher || - new Watcher({ - ui, - builder, - analytics: this.analytics, - options, - ignored: [path.resolve(this.project.root, options.outputPath)], - }); + ( + await Watcher.build({ + ui, + builder, + analytics: this.analytics, + options, + ignored: [path.resolve(this.project.root, options.outputPath)], + }) + ).watcher; await watcher; // Run until failure or signal to exit diff --git a/lib/tasks/serve.js b/lib/tasks/serve.js index 6f65eaed6c..e26bab3f1f 100644 --- a/lib/tasks/serve.js +++ b/lib/tasks/serve.js @@ -55,14 +55,16 @@ class ServeTask extends Task { let watcher = options._watcher || - new Watcher({ - ui: this.ui, - builder, - analytics: this.analytics, - options, - serving: true, - ignored: [path.resolve(this.project.root, options.outputPath)], - }); + ( + await Watcher.build({ + ui: this.ui, + builder, + analytics: this.analytics, + options, + serving: true, + ignored: [path.resolve(this.project.root, options.outputPath)], + }) + ).watcher; let serverRoot = './server'; let serverWatcher = null; diff --git a/lib/utilities/find-build-file.js b/lib/utilities/find-build-file.js index 1357bbd7e9..7cb3a5348f 100644 --- a/lib/utilities/find-build-file.js +++ b/lib/utilities/find-build-file.js @@ -1,24 +1,32 @@ 'use strict'; const findUp = require('find-up'); const path = require('path'); +const url = require('url'); -module.exports = function (file, dir) { - let buildFilePath = findUp.sync(file, { cwd: dir }); +module.exports = async function (dir) { + let buildFilePath = null; - // Note: In the future this should throw - if (!buildFilePath) { + for (let ext of ['js', 'mjs', 'cjs']) { + let candidate = findUp.sync(`ember-cli-build.${ext}`, { cwd: dir }); + if (candidate) { + buildFilePath = candidate; + break; + } + } + + if (buildFilePath === null) { return null; } process.chdir(path.dirname(buildFilePath)); - let buildFile = null; + let buildFileUrl = url.pathToFileURL(buildFilePath); try { - buildFile = require(buildFilePath); + // eslint-disable-next-line node/no-unsupported-features/es-syntax + let fn = (await import(buildFileUrl)).default; + return fn; } catch (err) { - err.message = `Could not require '${file}': ${err.message}`; + err.message = `Could not \`import('${buildFileUrl}')\`: ${err.message}`; throw err; } - - return buildFile; }; diff --git a/tests/acceptance/brocfile-smoke-test-slow.js b/tests/acceptance/brocfile-smoke-test-slow.js index 09db2fd9d9..bc4a3d5f94 100644 --- a/tests/acceptance/brocfile-smoke-test-slow.js +++ b/tests/acceptance/brocfile-smoke-test-slow.js @@ -61,6 +61,34 @@ describe('Acceptance: brocfile-smoke-test', function () { await runCommand(path.join('.', 'node_modules', 'ember-cli', 'bin', 'ember'), 'test'); }); + it('builds with an ES modules ember-cli-build.js', async function () { + await fs.writeFile( + 'ember-cli-build.js', + ` + import EmberApp from 'ember-cli/lib/broccoli/ember-app.js'; + + export default async function (defaults) { + const app = new EmberApp(defaults, { }); + + return app.toTree(); + }; + ` + ); + + let appPackageJson = await fs.readJson('package.json'); + appPackageJson.type = 'module'; + await fs.writeJson('package.json', appPackageJson); + + // lib/utilities/find-build-file.js uses await import and so can handle ES module ember-cli-build.js + // + // However, broccoli-config-loader uses require, so files like + // config/environment.js must be in commonjs format. The way to mix ES and + // commonjs formats in node is with multiple `package.json`s + await fs.writeJson('config/package.json', { type: 'commonjs' }); + console.log(process.cwd()); + await runCommand(path.join('.', 'node_modules', 'ember-cli', 'bin', 'ember'), 'build'); + }); + it('without app/templates', async function () { await copyFixtureFiles('brocfile-tests/pods-templates'); await fs.remove(path.join(process.cwd(), 'app/templates')); diff --git a/tests/fixtures/build/node-esm/app/hello.txt b/tests/fixtures/build/node-esm/app/hello.txt new file mode 100644 index 0000000000..802992c422 --- /dev/null +++ b/tests/fixtures/build/node-esm/app/hello.txt @@ -0,0 +1 @@ +Hello world diff --git a/tests/fixtures/build/node-esm/app/intro.md b/tests/fixtures/build/node-esm/app/intro.md new file mode 100644 index 0000000000..5dfd0725a4 --- /dev/null +++ b/tests/fixtures/build/node-esm/app/intro.md @@ -0,0 +1,3 @@ +# Introduction + +This is the introduction markdown file diff --git a/tests/fixtures/build/node-esm/app/outro.md b/tests/fixtures/build/node-esm/app/outro.md new file mode 100644 index 0000000000..1e7c322b6c --- /dev/null +++ b/tests/fixtures/build/node-esm/app/outro.md @@ -0,0 +1,3 @@ +# Outro + +This is the outro diff --git a/tests/fixtures/build/node-esm/app/test.txt b/tests/fixtures/build/node-esm/app/test.txt new file mode 100644 index 0000000000..0527e6bd2d --- /dev/null +++ b/tests/fixtures/build/node-esm/app/test.txt @@ -0,0 +1 @@ +This is a test diff --git a/tests/fixtures/build/node-esm/dist/intro.md b/tests/fixtures/build/node-esm/dist/intro.md new file mode 100644 index 0000000000..5dfd0725a4 --- /dev/null +++ b/tests/fixtures/build/node-esm/dist/intro.md @@ -0,0 +1,3 @@ +# Introduction + +This is the introduction markdown file diff --git a/tests/fixtures/build/node-esm/dist/outro.md b/tests/fixtures/build/node-esm/dist/outro.md new file mode 100644 index 0000000000..1e7c322b6c --- /dev/null +++ b/tests/fixtures/build/node-esm/dist/outro.md @@ -0,0 +1,3 @@ +# Outro + +This is the outro diff --git a/tests/fixtures/build/node-esm/dist/text.txt b/tests/fixtures/build/node-esm/dist/text.txt new file mode 100644 index 0000000000..531ac4dfd5 --- /dev/null +++ b/tests/fixtures/build/node-esm/dist/text.txt @@ -0,0 +1,3 @@ +Hello world + +This is a test diff --git a/tests/fixtures/build/node-esm/ember-cli-build.cjs b/tests/fixtures/build/node-esm/ember-cli-build.cjs new file mode 100644 index 0000000000..d3de00fe2f --- /dev/null +++ b/tests/fixtures/build/node-esm/ember-cli-build.cjs @@ -0,0 +1,17 @@ +const mergeTrees = require('broccoli-merge-trees'); +const concat = require('broccoli-concat'); +const funnel = require('broccoli-funnel'); + +module.exports = function() { + const txt = funnel('app', { + include: ['*.txt'], + }); + const md = funnel('app', { + include: ['*.md'], + }); + const concated = concat(txt, { + outputFile: 'text.txt', + }); + + return mergeTrees([concated, md], {annotation: 'The final merge'}); +} \ No newline at end of file diff --git a/tests/unit/models/builder-test.js b/tests/unit/models/builder-test.js index d92af85661..dd4983874f 100644 --- a/tests/unit/models/builder-test.js +++ b/tests/unit/models/builder-test.js @@ -21,7 +21,7 @@ let Builder; describe('models/builder.js', function () { let addon, builder, buildResults, tmpdir; - function setupBroccoliBuilder() { + async function setupBroccoliBuilder() { this.broccoliBuilderFallback = false; this.builder = { outputPath: 'build results', @@ -231,6 +231,26 @@ describe('models/builder.js', function () { expect(fixturify.readSync(result.directory)).to.deep.equal(fixturify.readSync(`${project.root}/dist`)); }); + // packages using node's module support (via type=module) need to have + // ember-cli-build.cjs rather than ember-cli.js in order for require to + // work correctly + it('builds packages using ESM', async function () { + const project = new MockProject(); + project.root += '/tests/fixtures/build/node-esm'; + const setupBuilder = () => + new Builder({ + project, + ui: project.ui, + copyToOutputPath() { + return []; + }, + }); + + let result = await setupBuilder().build(); + + expect(fixturify.readSync(result.directory)).to.deep.equal(fixturify.readSync(`${project.root}/dist`)); + }); + it('returns {directory, graph} as the result object', async function () { const project = new MockProject(); project.root += '/tests/fixtures/build/simple'; @@ -258,10 +278,15 @@ describe('models/builder.js', function () { project, ui: project.ui, setupBroccoliBuilder, + copyToOutputPath() { + return []; + }, }); }); it('is idempotent', async function () { + await builder.build(); + let cleanupCount = 0; builder.builder.cleanup = function () { cleanupCount++; @@ -308,8 +333,8 @@ describe('models/builder.js', function () { project.addons = [addon]; builder = new Builder({ - setupBroccoliBuilder() { - setupBroccoliBuilder.call(this); + async setupBroccoliBuilder() { + await setupBroccoliBuilder.call(this); let originalBuild = this.builder.build; this.builder.build = () => { hooksCalled.push('build'); @@ -425,6 +450,7 @@ describe('models/builder.js', function () { receivedBuildError = errorThrown; }; + await builder.setupBroccoliBuilder(); builder.builder.build = function () { hooksCalled.push('build'); @@ -447,6 +473,7 @@ describe('models/builder.js', function () { }); it('calls buildError and does not call postBuild or outputReady when build fails', async function () { + await builder.setupBroccoliBuilder(); builder.builder.build = function () { hooksCalled.push('build'); @@ -495,7 +522,7 @@ describe('models/builder.js', function () { }); describe('fallback from broccoli 2 to broccoli-builder', function () { - it('falls back to broccoli-builder if an InvalidNode error is thrown for read/rebuild api', function () { + it('falls back to broccoli-builder if an InvalidNode error is thrown for read/rebuild api', async function () { let project = new MockProject(); const builder = new Builder({ project, @@ -508,6 +535,7 @@ describe('models/builder.js', function () { }, }); + await builder.setupBroccoliBuilder(); expect(builder.broccoliBuilderFallback).to.be.true; expect(project.ui.output).to.include( @@ -521,15 +549,14 @@ describe('models/builder.js', function () { it('errors for an invalid node', function () { let project = new MockProject(); expect( - () => - new Builder({ - project, - ui: project.ui, - readBuildFile() { - return {}; - }, - }) - ).to.throw('[object Object] is not a Broccoli node\nused as output node'); + new Builder({ + project, + ui: project.ui, + readBuildFile() { + return {}; + }, + }).setupBroccoliBuilder() + ).to.be.rejectedWith('[object Object] is not a Broccoli node\nused as output node'); }); }); }); diff --git a/tests/unit/models/server-watcher-test.js b/tests/unit/models/server-watcher-test.js index 11b5ff88f4..ad6544cb5e 100644 --- a/tests/unit/models/server-watcher-test.js +++ b/tests/unit/models/server-watcher-test.js @@ -12,12 +12,12 @@ describe('Server Watcher', function () { let analytics; let watcher; - beforeEach(function () { + beforeEach(async function () { ui = new MockUI(); analytics = new MockAnalytics(); watcher = new MockServerWatcher(); - new ServerWatcher({ + await ServerWatcher.build({ ui, analytics, watcher, diff --git a/tests/unit/models/watcher-test.js b/tests/unit/models/watcher-test.js index 4f31df86af..a864dbce02 100644 --- a/tests/unit/models/watcher-test.js +++ b/tests/unit/models/watcher-test.js @@ -35,18 +35,20 @@ describe('Watcher', function () { }, }; - beforeEach(function () { + beforeEach(async function () { ui = new MockUI(); analytics = new MockAnalytics(); watcher = new MockBroccoliWatcher(); - subject = new Watcher({ - ui, - analytics, - builder, - watcher, - }); + subject = ( + await Watcher.build({ + ui, + analytics, + builder, + watcher, + }) + ).watcher; }); describe('watcher strategy selection', function () { @@ -136,29 +138,31 @@ describe('Watcher', function () { describe('output', function () { this.timeout(40000); - it('with ssl', function () { - let subject = new Watcher({ - ui, - analytics, - builder, - watcher, - serving: true, - options: { - host: undefined, - port: '1337', - ssl: true, - sslCert: 'tests/fixtures/ssl/server.crt', - sslKey: 'tests/fixtures/ssl/server.key', - environment: 'development', - project: { - config() { - return { - rootURL: '/', - }; + it('with ssl', async function () { + let subject = ( + await Watcher.build({ + ui, + analytics, + builder, + watcher, + serving: true, + options: { + host: undefined, + port: '1337', + ssl: true, + sslCert: 'tests/fixtures/ssl/server.crt', + sslKey: 'tests/fixtures/ssl/server.key', + environment: 'development', + project: { + config() { + return { + rootURL: '/', + }; + }, }, }, - }, - }); + }) + ).watcher; subject.didChange(mockResult); @@ -166,26 +170,28 @@ describe('Watcher', function () { expect(output[0]).to.equal(`${chalk.green('Build successful (12344ms)')} – Serving on https://localhost:1337/`); }); - it('with baseURL', function () { - let subject = new Watcher({ - ui, - analytics, - builder, - watcher, - serving: true, - options: { - host: undefined, - port: '1337', - environment: 'development', - project: { - config() { - return { - baseURL: '/foo', - }; + it('with baseURL', async function () { + let subject = ( + await Watcher.build({ + ui, + analytics, + builder, + watcher, + serving: true, + options: { + host: undefined, + port: '1337', + environment: 'development', + project: { + config() { + return { + baseURL: '/foo', + }; + }, }, }, - }, - }); + }) + ).watcher; subject.didChange(mockResult); @@ -196,26 +202,28 @@ describe('Watcher', function () { expect(output.length).to.equal(1, 'expected only one line of output'); }); - it('with rootURL', function () { - let subject = new Watcher({ - ui, - analytics, - builder, - watcher, - serving: true, - options: { - host: undefined, - port: '1337', - environment: 'development', - project: { - config() { - return { - rootURL: '/foo', - }; + it('with rootURL', async function () { + let subject = ( + await Watcher.build({ + ui, + analytics, + builder, + watcher, + serving: true, + options: { + host: undefined, + port: '1337', + environment: 'development', + project: { + config() { + return { + rootURL: '/foo', + }; + }, }, }, - }, - }); + }) + ).watcher; subject.didChange(mockResult); @@ -227,27 +235,29 @@ describe('Watcher', function () { expect(output.length).to.equal(1, 'expected only one line of output'); }); - it('with empty rootURL', function () { - let subject = new Watcher({ - ui, - analytics, - builder, - watcher, - serving: true, - options: { - host: undefined, - port: '1337', - rootURL: '', - environment: 'development', - project: { - config() { - return { - rootURL: '', - }; + it('with empty rootURL', async function () { + let subject = ( + await Watcher.build({ + ui, + analytics, + builder, + watcher, + serving: true, + options: { + host: undefined, + port: '1337', + rootURL: '', + environment: 'development', + project: { + config() { + return { + rootURL: '', + }; + }, }, }, - }, - }); + }) + ).watcher; subject.didChange(mockResult); @@ -256,27 +266,29 @@ describe('Watcher', function () { expect(output.length).to.equal(1, 'expected only one line of output'); }); - it('with customURL', function () { - let subject = new Watcher({ - ui, - analytics, - builder, - watcher, - serving: true, - options: { - host: undefined, - port: '1337', - rootURL: '', - environment: 'development', - project: { - config() { - return { - rootURL: '', - }; + it('with customURL', async function () { + let subject = ( + await Watcher.build({ + ui, + analytics, + builder, + watcher, + serving: true, + options: { + host: undefined, + port: '1337', + rootURL: '', + environment: 'development', + project: { + config() { + return { + rootURL: '', + }; + }, }, }, - }, - }); + }) + ).watcher; subject.serveURL = function () { return `http://customurl.com/`; }; diff --git a/tests/unit/utilities/find-build-file-test.js b/tests/unit/utilities/find-build-file-test.js index f1e9c2610f..abb871495e 100644 --- a/tests/unit/utilities/find-build-file-test.js +++ b/tests/unit/utilities/find-build-file-test.js @@ -1,46 +1,60 @@ 'use strict'; -const expect = require('chai').expect; +const expect = require('../../chai').expect; const fs = require('fs-extra'); +const os = require('os'); const path = require('path'); -const tmp = require('../../helpers/tmp'); const findBuildFile = require('../../../lib/utilities/find-build-file'); describe('find-build-file', function () { - let tmpPath = 'tmp/find-build-file-test'; + let tmpPath; let tmpFilename = 'ember-cli-build.js'; + let ROOT = process.cwd(); beforeEach(function () { - return tmp.setup(tmpPath).then(function () { - process.chdir(tmpPath); - }); + tmpPath = fs.mkdtempSync(path.join(os.tmpdir(), 'find-build-file-test')); + process.chdir(tmpPath); }); afterEach(function () { - let tmpFilePath = path.resolve(tmpFilename); - delete require.cache[require.resolve(tmpFilePath)]; - - return tmp.teardown(tmpPath); + process.chdir(ROOT); + fs.removeSync(tmpPath); }); - it('does not throw an error when the file is valid syntax', function () { + it('does not throw an error when the file is valid commonjs syntax', async function () { fs.writeFileSync(tmpFilename, "module.exports = function() {return {'a': 'A', 'b': 'B'};}", { encoding: 'utf8' }); - let result = findBuildFile(tmpFilename); + let result = await findBuildFile(); expect(result).to.be.a('function'); expect(result()).to.deep.equal({ a: 'A', b: 'B' }); }); - it('throws a SyntaxError if the file contains a syntax mistake', function () { - fs.writeFileSync(tmpFilename, "module.exports = function() {return {'a': 'A' 'b': 'B'};}", { encoding: 'utf8' }); + it('does not throw an error when the file is valid ES module syntax', async function () { + fs.writeFileSync('package.json', JSON.stringify({ type: 'module' }), { encoding: 'utf8' }); + fs.writeFileSync(tmpFilename, "export default function() {return {'a': 'A', 'b': 'B'};}", { encoding: 'utf8' }); + + let result = await findBuildFile(); + expect(result).to.be.a('function'); + expect(result()).to.deep.equal({ a: 'A', b: 'B' }); + }); + + it('throws a SyntaxError if the file contains a syntax mistake', async function () { + fs.writeFileSync(tmpFilename, 'module.exports = ', { encoding: 'utf8' }); + + let error = null; + try { + await findBuildFile(); + } catch (e) { + error = e; + } - expect(() => { - findBuildFile(tmpFilename); - }).to.throw(SyntaxError, /Could not require '.*':/); + expect(error).to.not.equal(null); + expect(error.constructor).to.equal(SyntaxError); + expect(error.message).to.match(/Could not `import\('.*ember-cli-build.*'\)`/); }); - it('does not throw an error when the file is mss', function () { - let result = findBuildFile('missing-file.js'); + it('does not throw an error when the file is missing', async function () { + let result = await findBuildFile(tmpPath); expect(result).to.be.null; }); });