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

Gulp Babili issue #454

Merged
merged 4 commits into from
Mar 7, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`gulp-babili comments should remove all comments when false 1`] = `"foo(),bar(),baz();"`;
exports[`gulp-babili comments should remove all comments when false 1`] = `"function foo(){}bar();var a=baz();"`;
Copy link
Member

Choose a reason for hiding this comment

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

converting foo() to function foo() {} is the weirdest thing to get it working

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes lol.. Dont even know how its working.. Just want to let you guys know there is something wrong!

Copy link
Member

Choose a reason for hiding this comment

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

So, when converting multiple statements to sequence expressions, the comments are dropped. Should add a test case that this doesn't affect the surrounding block comments.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes exactly.. add a separate test case?


exports[`gulp-babili comments should remove comments by default except license and preserve 1`] = `
/**
* @license
* This is a test
*/"foo(),bar(),baz();"`;
"/**
* @license
* This is a test
*/function foo(){}bar();var a=baz();"
`;

exports[`gulp-babili comments should take a custom function 1`] = `"/* YAC - yet another comment */foo(),bar(),baz();"`;
exports[`gulp-babili comments should take a custom function 1`] = `"function foo(){}bar();/* YAC - yet another comment */var a=baz();"`;

exports[`gulp-babili should remove comments while doing DCE and simplify 1`] = `
"var a=function(){(function(){foo(),bar(),baz();// comments should be optimized away
})()};"
`;
56 changes: 50 additions & 6 deletions packages/gulp-babili/__tests__/gulp-babili-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,11 @@ describe("gulp-babili", () => {
* @license
* This is a test
*/
foo();
function foo(){}
// this is another comment
bar();
/* YAC - yet another comment */
baz();
var a = baz();
`);

let file;
Expand All @@ -122,7 +122,7 @@ describe("gulp-babili", () => {
});
});

xit("should remove comments by default except license and preserve", () => {
it("should remove comments by default except license and preserve", () => {
return new Promise((resolve, reject) => {
const stream = gulpBabili();
stream.on("data", function (file) {
Expand All @@ -136,7 +136,7 @@ describe("gulp-babili", () => {

it("should remove all comments when false", () => {
return new Promise((resolve, reject) => {
const stream = gulpBabili({
const stream = gulpBabili({}, {
comments: false
});
stream.on("data", () => {
Expand All @@ -148,9 +148,9 @@ describe("gulp-babili", () => {
});
});

xit("should take a custom function", () => {
it("should take a custom function", () => {
return new Promise((resolve, reject) => {
const stream = gulpBabili({
const stream = gulpBabili({}, {
Copy link
Member Author

Choose a reason for hiding this comment

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

this is the options miss I was talking about @boopathi

comments(contents) {
return contents.indexOf("YAC") !== -1;
}
Expand All @@ -164,4 +164,48 @@ describe("gulp-babili", () => {
});
});
});

it("should remove comments while doing DCE and simplify", () => {
return new Promise((resolve, reject) => {
const stream = gulpBabili({}, {
comments(contents) {
return contents.indexOf("optimized") !== -1;
}
});

const source = unpad(`
/**
* @license
* throw away
*/
var a = function(){
// Hell yeah
function test(){
// comments should be optimized away
const flag = true;
if (flag) {
// comments
foo();
}
// remove this also
bar();
// should remove this as well
baz();
}
test();
}
`);

stream.on("data", function (file) {
expect(file.contents.toString()).toMatchSnapshot();
resolve();
});
stream.on("error", reject);

stream.write(new gutil.File({
path: "options.js",
contents: new Buffer(source)
}));
});
});
});