Breaking changes in 1.7.1

[ben-eb]

With the 1.7.1 update I'm seeing test failures, any ideas why this might be the case?

https://github.com/ben-eb/gulp-cssnano/pull/61
https://travis-ci.org/ben-eb/gulp-cssnano/jobs/166540992

[TerryMooreII]

After this package updated to 1.7.1, it now adds the word null between the minified js and comment for the source map.

... );null//# sourceMappingURL=../maps/scripts/dude-base.js.mapnull

This caused issues for us when we concated other minified javascript after the null, it just so happens that the concated js was an iffy so it turned the null to a call to a function called null. (If the minified js would have added a semicolon before the iffy we would have avoided this issue. Im looking at you angular).

Not sure if this is causing the issue that @ben-eb is seeing but it still shouldnt add the null.

[TerryMooreII]

Diving into the code it looks like after version 1.7.0 the code used a package called detect-newline to detect the new line character.
This line:
https://github.com/floridoo/gulp-sourcemaps/blob/v1.7.0/index.js#L269
Needs to be changed to
var newline = detectNewline(file.contents.toString()) || '\n';

And then updated for both the 1.7.x and 2.x builds.

[TerryMooreII]

Here is a pull request for this fix #231

[nmccready]

Thanks guys since this is relevant to both 1.X and 2.X we should target the PR for 1.X for easy roll up.

[AStoker]

Just throwing my hat into the lot here to make sure that this use case is accounted for in any PR's. We're running v2.0.0.
We have a function that looks like this:

function processSASS(){
    return gulp.src(project.cssProcessor.preCompiled) //Just an array [ "scss\\**\\*.scss" ]
     .pipe(sourcemaps.init())
     .pipe(sass({
         outputStyle: 'expanded'
     }).on('error', sass.logError))
     .pipe(sourcemaps.write())
     .pipe(concatCss('styles.css'))
     .pipe(gulp.dest('./wwwroot/dist'));
}

We have a mixins (scss/_mixins/mixin.js) file that's imported into partials (e.g. scss/_header.scss), and all those partials are imported into a single file (e.g. scss/base.scss) and all compiled into a single output file. Presently, what is happening is that sourcemaps appear to be created for the mixins file without any content (previously the mixins were ignored completely, now they are not).
What solved the issue for me (to at least get moving) was to rename _mixins/mixin.js to _/mixins/_mixin.js, adding the underscore before the name. It seems that previously if the folder path contained an underscore, it wouldn't create sourcemaps. Now it seems a bit more specific, saying only files that have an underscore won't create sourcemaps.
Hope that clarifies things a bit.

Related #226
Please move conversation there

[nmccready]

@AStoker this sounds like a possible different issue. Anyway to PR a spec to see this fail? Does #231 have any affect?

[AStoker]

It's possible @nmccready :) Spent a few hours today just trying to figure out why our builds were suddenly failing without any changes to code (found that this module was the one that was updated). I'm not entirely sure of all the expected behavior of partials with this module, all I know is that before I didn't need the underscore, and now I do :) It's possible that other people will have a similar setup (and therefore similar issues).
To be honest, my work load is a bit full, and I don't quite have time to do a PR and spec it out. The file probably needs to be named with an underscore anyway. I just wanted to document my problem and the solution so that other googler's will have an answer, and it seemed (initially) this would have been the place for it.

Related #226
Please move conversation there

[nmccready]

closing ea2e252