Skip to content

Commit

Permalink
Use await import in findBuildFile
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
hjdivad and rwjblue committed Oct 17, 2022
1 parent eac9d02 commit 6983c7f
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 50 deletions.
15 changes: 9 additions & 6 deletions lib/models/builder.js
Expand Up @@ -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);
Expand All @@ -42,11 +41,11 @@ 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.');
Expand All @@ -56,11 +55,11 @@ class Builder extends CoreObject {
* @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');

Expand Down Expand Up @@ -167,6 +166,10 @@ class Builder extends CoreObject {
* @return {Promise}
*/
async build(addWatchDirCallback, resultAnnotation) {
if (this.builder === undefined) {
await this.setupBroccoliBuilder();
}

let buildResults, uiProgressIntervalID;

try {
Expand Down
23 changes: 12 additions & 11 deletions lib/utilities/find-build-file.js
Expand Up @@ -2,28 +2,29 @@
const findUp = require('find-up');
const path = require('path');

// 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;
try {
buildFile = require(buildFilePath);
// eslint-disable-next-line node/no-unsupported-features/es-syntax
let fn = (await import(buildFilePath)).default;
return fn;
} catch (err) {
err.message = `Could not require 'ember-cli-build.js': ${err.message}`;
throw err;
}

return buildFile;
};
28 changes: 28 additions & 0 deletions tests/acceptance/brocfile-smoke-test-slow.js
Expand Up @@ -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'));
Expand Down
33 changes: 20 additions & 13 deletions tests/unit/models/builder-test.js
Expand Up @@ -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',
Expand Down Expand Up @@ -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++;
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -445,6 +450,7 @@ describe('models/builder.js', function () {
receivedBuildError = errorThrown;
};

await builder.setupBroccoliBuilder();
builder.builder.build = function () {
hooksCalled.push('build');

Expand All @@ -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');

Expand Down Expand Up @@ -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,
Expand All @@ -528,6 +535,7 @@ describe('models/builder.js', function () {
},
});

await builder.setupBroccoliBuilder();
expect(builder.broccoliBuilderFallback).to.be.true;

expect(project.ui.output).to.include(
Expand All @@ -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');
});
});
});
53 changes: 33 additions & 20 deletions tests/unit/utilities/find-build-file-test.js
@@ -1,46 +1,59 @@
'use strict';

const expect = require('chai').expect;
const expect = require('../../chai').expect;
const fs = require('fs-extra');
const path = require('path');
const tmp = require('../../helpers/tmp');
const os = require('os');
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(os.tmpdir());
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();
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();
}).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 require 'ember-cli-build.js'/);
});

it('does not throw an error when the file is missing', function () {
let result = findBuildFile(tmpPath);
it('does not throw an error when the file is missing', async function () {
let result = await findBuildFile(tmpPath);
expect(result).to.be.null;
});
});

0 comments on commit 6983c7f

Please sign in to comment.