Skip to content

add shorthands to builder #151

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions lib/builder.js
Original file line number Diff line number Diff line change
@@ -3,9 +3,10 @@
* Module dependencies.
*/

var lookup = require('./lookup');
var Build = require('./build');
var Batch = require('batch');
var Build = require('./build');
var lookup = require('./lookup');
var shorthands = require('./shorthands');

/**
* Expose `Builder`
@@ -97,3 +98,9 @@ Builder.prototype.build = function(fn){
});
});
};

/**
* Mixin shorthands.
*/

for (var key in shorthands) Builder.prototype[key] = shorthands[key];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

proto ftw

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or do we use that already above somewhere

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nah we dont i dont think. seems weird though to set the shorthands as the proto?

126 changes: 126 additions & 0 deletions lib/shorthands.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@

var commonjs = require('./plugins/commonjs');
var concat = require('./plugins/concat');
var copy = require('./plugins/copy');
var rewriteUrls = require('./plugins/rewrite-urls');
var symlink = require('./plugins/symlink');

/**
* Build all the native types with an `out` directory and optional `options`
* then invoke `fn(err, build)`.
*
* @param {String} out
* @param {Object} options (optional)
* @property {Boolean} symlink
* @property {String} rewriteUrls
* @param {Function} fn
*/

exports.all = function (out, options, fn) {
if ('function' == typeof options) fn = options, options = null;
options = options || {};

this
.use(commonjs('json'))
.use(commonjs('scripts'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think these three need to be in commonjs() as a plugin, we have to build a require map beforehand to get rid of aliasing so having them as separate plugins wont work to well, unless we abstracted even more but that just gets confusing if the plugins have weird relationships

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, i didn't understand that. let me know what you mean and i can hopefully make the change, or @yields can

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

started it the other day but got side tracked haha. The first thing is I think that since these are defined by spec, we should need less plugins to make them work, just commonjs() vs each individual one. The other thing is that in order to get rid of aliases we need to first construct a map of all the require()ables, otherwise we can't resolve before outputting javascript, but then we can ditch those bloaty things

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotcha. so for the first part, my thought was that the shorthands in this PR were actually how we "conforming with the spec". as in it makes sense for having each one be built of smaller pieces like commonjs and concat but then that the spec comes into play by making these shorthands ease to access. does that make sense?

oh! what if we changed this so that the shorthand methods just pushed the plugins onto the current build stack instead of actually ending the batch as well. so then using them would be like this:

builder
  .scripts()
  .styles()
  .build(function(err, build){
    ...
  });

which means that using them is super easy that way (don't even have to reach into things exposed on the static like Builder.scripts or Builder.concat which is kinda ugly feeling anyways) and so that adding more coffeescripts or something would be simple as:

builder
  .scripts()
  .use(coffee())
  .build(fn);

but then the scripts itself can still be composed of smaller plugins like concat('scripts'). and we wouldn't expose those plugins on the builder at all, because they'd be in separate repos that people can just require in instead.

that doesn't solve any of the require stuff haha. but for the require things, i think we're going to need a different solution anyways, because even just adding .coffee files to get compiled would also need to be part of the same system right?

can we just have a thing where instead of getting catd together they just get pushed onto an array somewhere that the builder knows about?

.use(commonjs('templates'))
.use(rewrite(options.rewriteUrls || ''))
.use(concat('json'))
.use(concat('scripts'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

concat() should be one as well IMO

.use(concat('styles'))
.use(concat('templates'))
.use(copy('fonts', out, options))
.use(copy('images', out, options))
.build(fn);
};

/**
* Build scripts and invoke `fn(err, build)`.
*
* @param {Function} fn
*/

exports.scripts = function (fn) {
this
.use(commonjs('scripts'))
.use(concat('scripts'))
.build(fn);
};

/**
* Build styles with `options` and invoke `fn(err, build)`.
*
* @param {Object} options (optional)
* @property {String} rewriteUrls
* @param {Function} fn
*/

exports.styles = function (options, fn) {
if ('function' == typeof options) fn = options, options = null;
options = options || {};

this
.use(rewriteUrls(options.rewriteUrls || ''))
.use(concat('styles'))
.build(fn);
};

/**
* Build templates and invoke `fn(err, build)`.
*
* @param {Function} fn
*/

exports.templates = function (fn) {
this
.use(commonjs('templates'))
.use(concat('templates'))
.build(fn);
};

/**
* Build json and invoke `fn(err, build)`.
*
* @param {Function} fn
*/

exports.json = function (fn) {
this
.use(commonjs('json'))
.use(concat('json'))
.build(fn);
};

/**
* Build images with `out` directory and `options` and invoke `fn(err, build)`.
*
* @param {String} out
* @param {Object} options (optional)
* @param {Function} fn
*/

exports.images = function (out, options, fn) {
if ('function' == typeof options) fn = options, options = null;
options = options || {};

this
.use(copy('images', out, options))
.build(fn);
};

/**
* Build fonts with `out` directory and `options` and invoke `fn(err, build)`.
*
* @param {String} out
* @param {Object} options (optional)
* @param {Function} fn
*/

exports.fonts = function (out, options, fn) {
if ('function' == typeof options) fn = options, options = null;
options = options || {};

this
.use(copy('fonts', out, options))
.build(fn);
};
10 changes: 10 additions & 0 deletions test/fixtures/fonts/component.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"name": "fonts",
"description": "a bunch of fonts",
"styles": [
"style.css"
],
"fonts": [
"montserrat-regular.ttf"
]
}
Binary file added test/fixtures/fonts/montserrat-regular.ttf
Binary file not shown.
6 changes: 6 additions & 0 deletions test/fixtures/fonts/style.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
@font-face {
font-family: 'Montserrat';
src: url('fonts/montserrat-regular.ttf') format('truetype');
font-weight: normal;
font-style: normal;
}
147 changes: 147 additions & 0 deletions test/shorthands.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@

var Builder = require('..');
var exec = require('child_process').exec;
var exists = require('fs').existsSync;
var fs = require('fs');
var read = require('fs').readFileSync;
var vm = require('vm');

describe('shorthands', function(){
afterEach(function(done){
exec('rm -rf /tmp/build', done);
});

describe('scripts', function(){
it('should build scripts', function(done){
var builder = new Builder('test/fixtures/hello');
builder.path('..');
builder.scripts(function(err, build){
if (err) return done(err);
var js = build.scripts.trim();
var out = read('test/fixtures/hello-js.js', 'utf8').trim();
js.should.eql(out);
done();
});
});
});

describe('json', function(){
it('should build json', function(done){
var builder = new Builder('test/fixtures/json');
builder.path('..');
builder.json(function(err, build){
if (err) return done(err);
var js = build.json.trim();
var out = read('test/fixtures/json.js', 'utf8').trim();
js.should.eql(out);
done();
});
});
});

describe('templates', function(){
it('should build templates', function(done){
var builder = new Builder('test/fixtures/template-strings');
builder.path('..');
builder.templates(function(err, build){
if (err) return done(err);
var js = build.templates.trim();
var out = read('test/fixtures/template.js', 'utf8').trim();
js.should.eql(out);
done();
});
});
});

describe('styles', function(){
it('should build styles', function(done){
var builder = new Builder('test/fixtures/assets-parent');
builder.path('..');
builder.styles(function(err, build){
if (err) return done(err);
build.styles.should.include('url("assets/images/logo.png")');
build.styles.should.include('url("assets/images/maru.jpeg")');
build.styles.should.include('url("assets/images/npm.png")');
build.styles.should.include('url(http://example.com/images/manny.png)');
build.styles.should.include('url(/public/images/foo.png)')
build.styles.should.include('url(data:image/png;base64,PNG DATA HERE)');
done();
});
});

it('should rewrite urls', function(done){
var builder = new Builder('test/fixtures/assets-parent');
builder.path('..');
builder.styles({ rewriteUrls: 'build' }, function(err, build){
if (err) return done(err);
build.styles.should.include('url("build/assets/images/logo.png")');
build.styles.should.include('url("build/assets/images/maru.jpeg")');
build.styles.should.include('url("build/assets/images/npm.png")');
build.styles.should.include('url(http://example.com/images/manny.png)');
build.styles.should.include('url(/public/images/foo.png)')
build.styles.should.include('url(data:image/png;base64,PNG DATA HERE)');
done();
});

});
});

describe('images', function(){
it('should build images', function(done){
var builder = new Builder('test/fixtures/assets');
builder.path('..');
builder.images('/tmp/build', function(err){
if (err) return done(err);
exists('/tmp/build/assets/images/maru.jpeg').should.be.true;
exists('/tmp/build/assets/images/logo.png').should.be.true;
exists('/tmp/build/assets/images/npm.png').should.be.true;
fs.lstat('/tmp/build/assets/images/npm.png', function(err, stats) {
if (err) return done(err);
stats.isSymbolicLink().should.be.false;
done();
});
});
});

it('should symlink when option is true', function(done){
var builder = new Builder('test/fixtures/assets');
builder.path('..');
builder.images('/tmp/build', { symlink: true }, function(err){
if (err) return done(err);
fs.lstat('/tmp/build/assets/images/npm.png', function(err, stats) {
if (err) return done(err);
stats.isSymbolicLink().should.be.true;
done();
});
});
});
});

describe('fonts', function(){
it('should build fonts', function(done){
var builder = new Builder('test/fixtures/fonts');
builder.path('..');
builder.fonts('/tmp/build', function(err){
if (err) return done(err);
fs.lstat('/tmp/build/fonts/montserrat-regular.ttf', function(err, stats) {
if (err) return done(err);
stats.isSymbolicLink().should.be.false;
done();
});
});
});

it('should symlink when option is true', function(done){
var builder = new Builder('test/fixtures/fonts');
builder.path('..');
builder.fonts('/tmp/build', { symlink: true }, function(err){
if (err) return done(err);
fs.lstat('/tmp/build/fonts/montserrat-regular.ttf', function(err, stats) {
if (err) return done(err);
stats.isSymbolicLink().should.be.true;
done();
});
});
});
});
});