Use await import in findBuildFile

Also await the actual build function, so ember-cli-build.js can export
an async builder function.

Co-authored-by: Robert Jackson <rjackson@linkedin.com>

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(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 {

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));

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 });

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

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;

lib/utilities/find-build-file.js

@@ -1,29 +1,32 @@
 'use strict';
 const findUp = require('find-up');
 const path = require('path');
+const url = require('url');
 
-// TODO: make rob happy with async import
-module.exports = function (dir) {
-  let buildFilePath = findUp.sync('ember-cli-build.js', { cwd: dir });
+module.exports = async function (dir) {
+  let buildFilePath = null;
 
-  if (!buildFilePath) {
-    buildFilePath = findUp.sync('ember-cli-build.cjs', { cwd: dir });
+  for (let ext of ['js', 'mjs', 'cjs']) {
+    let candidate = findUp.sync(`ember-cli-build.${ext}`, { cwd: dir });
+    if (candidate) {
+      buildFilePath = candidate;
+      break;
+    }
   }
 
-  // Note: In the future this should throw
-  if (!buildFilePath) {
+  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 'ember-cli-build.js': ${err.message}`;
+    err.message = `Could not \`import('${buildFileUrl}')\`: ${err.message}`;
     throw err;
   }
-
-  return buildFile;
 };

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'));

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',
@@ -278,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++;
@@ -328,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');
@@ -445,6 +450,7 @@ describe('models/builder.js', function () {
         receivedBuildError = errorThrown;
       };
 
+      await builder.setupBroccoliBuilder();
       builder.builder.build = function () {
         hooksCalled.push('build');
 
@@ -467,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');
 
@@ -515,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,
@@ -528,6 +535,7 @@ describe('models/builder.js', function () {
         },
       });
 
+      await builder.setupBroccoliBuilder();
       expect(builder.broccoliBuilderFallback).to.be.true;
 
       expect(project.ui.output).to.include(
@@ -541,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');
     });
   });
 });

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,