search

gulp-sourcemaps / vinyl-sourcemaps-apply

Helper for implementing source map support in Gulp plugins
19 stars 8 forks source link

v0.1.2 is a breaking change #2

Closed englercj closed 10 years ago

englercj commented 10 years ago

Looks like this commit that adds assertions is actually a breaking change:

1bee9f597cb9d89469f8b8f428ff30c4b7c832fd

Now gulp-less no longer works since it was (and still is) not passing a "file" property. Took me a while to track this down.

I was able to work around by using specifically 0.1.1 in my package.json which causes npm to dedupe the deep dependency on this library in gulp-less and use my version I install; so it works. But really this should have bumped the major version since it is not backwards compat.

jvdamgaard commented 10 years ago

I'm having the same issue with gulp-less (version 1.3.5) which has this dependency:

"vinyl-sourcemaps-apply": "^0.1.1"

The error is something like:

node_modules/gulp-less/node_modules/vinyl-source-maps-apply/index.js:33
    throw e:
Error: Source map to be applied is missing the "file" property
    at assertProperty (node_modules/gulp-less/node_modules/vinyl-source-maps-apply/index.js:33:13)
    at applySourceMap (node_modules/gulp-less/node_modules/vinyl-source-maps-apply/index.js:11:3)
    at node_modules/gulp-less/index.js:65:13
    ...
floridoo commented 10 years ago

gulp-less has a bug making the plugin following after it crash under certain circumstances. Because of this the issue was filed in different plugins even though gulp-less caused the error, causing some confusion.

Now vinyl-sourcemaps-apply checks for missing source map properties before applying the source map, so the error is being thrown in the plugin really causing it, gulp-less in your case.

englercj commented 10 years ago

Sounds like a good fix, but I wasn't arguing the merits of the change; just that it is a breaking change.

We were no longer able to build when our jenkins server updated to a new patch version of vinyl-sourcemaps-apply (due to gulp-less asking for ^0.1.1) because of this change. My build should never fail when updating a patch version, or that is a violation of the semver spec.

Looks like this was a good change to make, but it needed to update the major version. This patch is not backwards compatible.

floridoo commented 10 years ago

You are right, it should have been a major version update. Sorry for that.