Skip to content

Commit

Permalink
Changed build option -> bundle option and using build option to contr…
Browse files Browse the repository at this point in the history
…ol whether the assets are stored to disk, matching the original intent.
  • Loading branch information
blakevanlan committed Apr 10, 2016
1 parent 461857a commit 458fc81
Show file tree
Hide file tree
Showing 11 changed files with 56 additions and 32 deletions.
1 change: 1 addition & 0 deletions .gitignore
Expand Up @@ -5,3 +5,4 @@ npm-debug.log
builtAssets/
testBuiltAssets/
testBuiltAssetsFoo/
example/
7 changes: 6 additions & 1 deletion CHANGELOG.md
Expand Up @@ -6,7 +6,12 @@
* `csswring`: `3.0.5` -> `4.2.2`
* `mincer`: `1.3.0` -> `1.4.1`
* `uglify-js: `2.4.23` -> `2.6.2`
* `postcss`: `5.0.19` (new dependency)
* `postcss`: `5.0.19` (new dependency)
* Changes `build` option to correctly manage whether the assets are saved to disk
* Adds `bundle` option to manage whether the assets are bundled under a single tag (used to actually be controlled by the `build` option)

### Upgrade Notes
For the majority of people, the changes to `build` option and the addition of `bundle` option should "just work". However, just providing a `buildDir` will no longer start saving assets to disk; the `build` option must also be enabled.

## 5.1.1
* Fix for compile=true ignoring already built asset (#322), thanks to @dapriett
Expand Down
3 changes: 2 additions & 1 deletion README.md
Expand Up @@ -145,8 +145,9 @@ helperContext | global | The object that helper functio
servePath | "assets" | The virtual path in which assets will be served over HTTP. If hosting assets locally, supply a local path (say, "assets"). If hosting assets remotely on a CDN, supply a URL: "http://myassets.example.com/assets".
precompile | ["\*.\*"] | An array of assets to precompile while the server is initializing. Patterns should match the filename only, not including the directory.
build | dev: false; prod: true | Should assets be saved to disk (true), or just served from memory (false)?
buildDir | dev: false; prod: "builtAssets" | The directory to save (and load) compiled assets to/from.
buildDir | "builtAssets" | The directory to save (and load) compiled assets to/from.
compile | true | Should assets be compiled if they don’t already exist in the `buildDir`?
bundle | dev: false; prod: true | Should assets be bundled into a single tag (when possible)?
compress | dev: false; prod: true | Should assets be minified? If enabled, requires `uglify-js` and `csswring`.
gzip | false | Should assets have gzipped copies in `buildDir`?
fingerprinting| dev: false; prod: true | Should fingerprints be appended to asset filenames?
Expand Down
1 change: 1 addition & 0 deletions bin/connect-assets
Expand Up @@ -126,6 +126,7 @@ var _compile = exports._compile = function (args, callback) {
compile: true,
compress: true,
gzip: args.gzip,
build: true,
buildDir: args.output,
servePath: args.servePath,
precompile: args.compile,
Expand Down
3 changes: 2 additions & 1 deletion index.js
Expand Up @@ -70,8 +70,9 @@ var parseOptions = module.exports._parseOptions = function (options) {
options.localServePath = options.localServePath || servePathPathname.replace(/^\//, "");
options.precompile = arrayify(options.precompile || ["*.*"]);
options.build = options.build != null ? options.build : isProduction;
options.buildDir = options.buildDir != null ? options.buildDir : isDevelopment ? false : "builtAssets";
options.buildDir = options.buildDir != null ? options.buildDir : "builtAssets";
options.compile = options.compile != null ? options.compile : true;
options.bundle = options.bundle != null ? options.bundle : isProduction;
options.compress = options.compress != null ? options.compress : isProduction;
options.sourceMaps = options.sourceMaps != null ? options.sourceMaps : isDevelopment;
options.gzip = options.gzip != null ? options.gzip : false;
Expand Down
8 changes: 4 additions & 4 deletions lib/assets.js
Expand Up @@ -30,7 +30,7 @@ var Assets = module.exports = function (Mincer, options) {

this.options.paths.forEach(this.environment.appendPath, this.environment);

if (this.options.buildDir) {
if (this.options.build) {
this.manifest = new Mincer.Manifest(this.environment, this.options.buildDir);
}
}
Expand Down Expand Up @@ -86,11 +86,11 @@ Assets.prototype.serveAsset = function (req, res, next) {
var asset, assetName;

try {
asset = this.getAssetByPath(parts.path, { bundle: this.options.build });
asset = this.getAssetByPath(parts.path, { bundle: this.options.bundle });

if (isPossibleSourceMapPath(path) && !asset) {
var originalAssetPath = parts.path.substring(0, parts.path.length - 4);
asset = this.getAssetByPath(originalAssetPath, { bundle: this.options.build });
asset = this.getAssetByPath(originalAssetPath, { bundle: this.options.bundle });
if (!asset || !asset.sourceMap) {
return next();
}
Expand Down Expand Up @@ -198,7 +198,7 @@ Assets.prototype.helper = function (tagWriter, ext) {
return tagWriter(path, attributes);
};

if (!instance.options.build && asset.type === "bundled") {
if (!instance.options.bundle && asset.type === "bundled") {
var assets = asset.toArray();
var tags = [];

Expand Down
39 changes: 27 additions & 12 deletions test/parseOptions.js
Expand Up @@ -135,20 +135,9 @@ describe("parseOptions", function () {
});

describe("buildDir", function () {
it("defaults to false in development", function () {
var env = process.env.NODE_ENV;
process.env.NODE_ENV = "development";
var opts = assets._parseOptions({});
expect(opts.buildDir).to.equal(false);
process.env.NODE_ENV = env;
});

it("defaults to 'builtAssets' in production", function () {
var env = process.env.NODE_ENV;
process.env.NODE_ENV = "production";
it("defaults to 'builtAssets'", function () {
var opts = assets._parseOptions({});
expect(opts.buildDir).to.equal("builtAssets");
process.env.NODE_ENV = env;
});

it("can be overridden", function () {
Expand Down Expand Up @@ -179,6 +168,32 @@ describe("parseOptions", function () {
});
});

describe("bundle", function () {
it("defaults to false in development", function () {
var env = process.env.NODE_ENV;
process.env.NODE_ENV = "development";
var opts = assets._parseOptions({});
expect(opts.bundle).to.equal(false);
process.env.NODE_ENV = env;
});

it("defaults to true in production", function () {
var env = process.env.NODE_ENV;
process.env.NODE_ENV = "production";
var opts = assets._parseOptions({});
expect(opts.bundle).to.equal(true);
process.env.NODE_ENV = env;
});

it("can be overridden", function () {
var env = process.env.NODE_ENV;
process.env.NODE_ENV = "development";
var opts = assets._parseOptions({ bundle: true });
expect(opts.bundle).to.equal(true);
process.env.NODE_ENV = env;
});
});

describe("compress", function () {
it("defaults to false in development", function () {
var env = process.env.NODE_ENV;
Expand Down
2 changes: 1 addition & 1 deletion test/serveAsset.asset_path-helper.js
Expand Up @@ -11,7 +11,7 @@ describe("serveAsset asset_path environment helper", function () {
var env = process.env.NODE_ENV;
var dir = "testBuiltAssetsFoo";

createServer.call(this, { buildDir: dir, compile: true, fingerprinting: true }, function () {
createServer.call(this, { build: true, buildDir: dir, compile: true, fingerprinting: true }, function () {
var path = this.assetPath("asset-path-helper.css");
var filename = dir + "/asset-path-helper-ecc5d891145d56a0274eb4f34670671e.css";
var url = this.host + path;
Expand Down
2 changes: 1 addition & 1 deletion test/serveAsset.build.js → test/serveAsset.bundle.js
Expand Up @@ -5,7 +5,7 @@ var fs = require("fs");
var createServer = require("./testHelpers/createServer");
var rmrf = require("./testHelpers/rmrf");

describe("serveAsset build", function () {
describe("serveAsset bundle", function () {

before(function () {
this.env = process.env.NODE_ENV;
Expand Down
18 changes: 9 additions & 9 deletions test/serveAsset.manifest.js
Expand Up @@ -10,7 +10,7 @@ describe("serveAsset manifest", function () {
it("outputs a manifest if it does not exist", function (done) {
var dir = "testBuiltAssets";

createServer.call(this, { buildDir: dir }, function () {
createServer.call(this, { build: true, buildDir: dir }, function () {
var path = this.assetPath("blank.css");
var url = this.host + path;

Expand All @@ -30,7 +30,7 @@ describe("serveAsset manifest", function () {
fs.mkdirSync(dir);
fs.writeFileSync(dir + "/manifest.json", "{}");

createServer.call(this, { buildDir: dir, compile: false }, function () {
createServer.call(this, { build: true, buildDir: dir, compile: false }, function () {
expect(fs.readFileSync(dir + "/manifest.json", "utf8")).to.equal("{}");
rmrf(dir, done);
});
Expand All @@ -42,7 +42,7 @@ describe("serveAsset manifest", function () {
var contents = "html {}";
fs.writeFileSync(file, contents);

createServer.call(this, { buildDir: dir, compile: true }, function () {
createServer.call(this, { build: true, buildDir: dir, compile: true }, function () {
var path = this.assetPath("will-change.css");
var url = this.host + path;
fs.writeFileSync(file, "body {}");
Expand All @@ -62,7 +62,7 @@ describe("serveAsset manifest", function () {
it("serves files outside of the manifest if compile is true", function (done) {
var dir = "testBuiltAssets";

createServer.call(this, { buildDir: dir, compile: true }, function () {
createServer.call(this, { build: true, buildDir: dir, compile: true }, function () {
var path = this.assetPath("blank.css");
var url = this.host + path;

Expand Down Expand Up @@ -102,7 +102,7 @@ describe("serveAsset manifest", function () {
try { fs.unlinkSync(file); }
catch (e) { if (e.code != "ENOENT") throw e; }

createServer.call(this, { buildDir: dir, compile: true }, function () {
createServer.call(this, { build: true, buildDir: dir, compile: true }, function () {
var path = this.assetPath("blank.css");
var url = this.host + path;

Expand Down Expand Up @@ -140,14 +140,14 @@ describe("serveAsset manifest", function () {
if (err && err.code != "ENOENT") return done(err);

// Build the initial manifest.
createServer.call(this, { buildDir: dir, compile: true }, function () {
createServer.call(this, { build: true, buildDir: dir, compile: true }, function () {
var path = this.assetPath("blank.css");
var url = this.host + path;

http.get(url, function (res) {

// Now, create a server using the existing manifest.
createServer.call(this, { buildDir: dir, compile: false }, function () {
createServer.call(this, { build: true, buildDir: dir, compile: false }, function () {
var file = "test/assets/css/new-file-manifest-no-compile.css";
fs.writeFileSync(file, "");

Expand All @@ -170,7 +170,7 @@ describe("serveAsset manifest", function () {
if (err && err.code != "ENOENT") return done(err);

// Build the initial manifest.
createServer.call(this, { buildDir: dir, compile: true }, function () {
createServer.call(this, { build: true, buildDir: dir, compile: true }, function () {
var path = this.assetPath("blank.css");
var url = this.host + path;

Expand All @@ -180,7 +180,7 @@ describe("serveAsset manifest", function () {
delete require.cache[require.resolve(nodePath.join(nodePath.resolve(dir), 'manifest.json'))];

// Now, create a server using the existing manifest.
createServer.call(this, { buildDir: dir, compile: false, servePath: "http://cdn.example.com/assets" }, function () {
createServer.call(this, { build: true, buildDir: dir, compile: false, servePath: "http://cdn.example.com/assets" }, function () {
path = this.assetPath("blank.css");
url = this.host + path.replace("http://cdn.example.com", "");

Expand Down
4 changes: 2 additions & 2 deletions test/serveAsset.sourcemaps.js
Expand Up @@ -8,7 +8,7 @@ var rmrf = require("./testHelpers/rmrf");
describe("serveAsset sourcemaps", function () {

it("adds source map header to asset", function (done) {
createServer.call(this, { build: true, compress: true, sourceMaps: true }, function () {
createServer.call(this, { bundle: true, compress: true, sourceMaps: true }, function () {
var path = this.assetPath("unminified.js");
var url = this.host + path;
var sourceMapPath = path + ".map";
Expand All @@ -22,7 +22,7 @@ describe("serveAsset sourcemaps", function () {
});

it("serves source map", function (done) {
createServer.call(this, { build: true, compress: true, sourceMaps: true }, function () {
createServer.call(this, { bundle: true, compress: true, sourceMaps: true }, function () {
var path = this.assetPath("unminified.js");
var url = this.host + path + ".map";

Expand Down

0 comments on commit 458fc81

Please sign in to comment.