Skip to content
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

[PATCH]Read synchronously for larger files while concatenating #1

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
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
6 changes: 5 additions & 1 deletion concat.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ class Concat extends Plugin {
this.footer = options.footer;
this.footerFiles = options.footerFiles;
this.separator = (options.separator != null) ? options.separator : '\n';
this.contentLimit = options.contentLimit || 10000;
this.inputNode = inputNode;

ensureNoGlob('headerFiles', this.headerFiles);
ensureNoGlob('footerFiles', this.footerFiles);
Expand Down Expand Up @@ -110,7 +112,9 @@ class Concat extends Plugin {
header: this.header,
headerFiles: this.headerFiles,
footerFiles: this.footerFiles,
footer: this.footer
footer: this.footer,
contentLimit: this.contentLimit,
baseDir: this.inputNode
}));
}

Expand Down
33 changes: 30 additions & 3 deletions lib/strategies/simple.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
'use strict';

var findIndex = require('find-index');
var fs = require('fs');
var baseDir;

function contentMapper(entry) {
if (entry.isFileContentLarge) {
entry.content = fs.readFileSync(baseDir + entry.file, 'utf-8');
}
return entry.content;
}

Expand All @@ -18,6 +23,8 @@ class SimpleConcat {
this.headerFiles = attrs.headerFiles || [];
this.footerFiles = attrs.footerFiles || [];
this.footer = attrs.footer;
this.contentLimit = attrs.contentLimit;
baseDir = attrs.baseDir ? attrs.baseDir + '/' : '';

// Internally, we represent the concatenation as a series of entries. These
// entries have a 'file' attribute for lookup/sorting and a 'content' property
Expand All @@ -37,6 +44,14 @@ class SimpleConcat {
return findIndex(this._internal, function(entry) { return entry.file === file; });
}

/**
* Check if the content of the file is too large to be stored in memory
*/
_isContentLarge(content) {
var contentLimit = this.contentLimit;
return contentLimit && content.length > contentLimit;
}

/**
* Updates the contents of a header file.
*/
Expand Down Expand Up @@ -77,10 +92,16 @@ class SimpleConcat {
}

var entry = {
file: file,
content: content
file: file
};

if (this._isContentLarge(content)) {
entry.content = '';
entry.isFileContentLarge = true;
} else {
entry.content = content;
}

var index = findIndex(this._internal, function(entry) { return entry.file > file; });
if (index === -1) {
this._internal.push(entry);
Expand All @@ -100,7 +121,13 @@ class SimpleConcat {
throw new Error('Trying to update ' + file + ' but it has not been read before');
}

this._internal[index].content = content;
if (this._isContentLarge(content)) {
this._internal[index].content = '';
this._internal[index].isFileContentLarge = true;
} else {
this._internal[index].content = content;
}

}

removeFile(file) {
Expand Down
12 changes: 12 additions & 0 deletions test/simple-concat-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,18 @@ describe('simple-concat', function() {
expectFile('no-sourcemap.map').notIn(result);
}));

it('concatenates files with content higher than limit', co.wrap(function* () {
var node = concat(firstFixture, {
outputFile: '/all-inner.js',
inputFiles: ['inner/*.js'],
contentLimit: 10,
sourceMapConfig: { enabled: false }
});
builder = new broccoli.Builder(node);
let result = yield builder.build();
expectFile('all-inner.js').withoutSrcURL().in(result);
}));

/**
* Tests below here should appear for both simple-concat and sourcemap-concat.
*/
Expand Down