search

timse / srcset-loader

Flexible srcset-loader for webpack
46 stars 14 forks source link

Upgrade sharp #13

Closed dcalhoun closed 7 years ago

dcalhoun commented 7 years ago

Upgrade sharp to supress libvips warnings, namely the following.

.../node_modules/good-listener/src/is.jsvips warning: VipsJpeg: error reading resolution && vips warning: VipsJpeg: error reading resolution

Related: lovell/sharp#657

dcalhoun commented 7 years ago

@timse there is a single test failing in the master branch. Do you happen to know why?

What do you think about adding a .travis.yml to track the build for this repository?

timse commented 7 years ago

Hi sorry for the very very late reply - happy to add a .travel.yml - would you mind doing so? Happy to merge

dcalhoun commented 7 years ago

@timse I've opened #19 to add Travis CI support.

timse commented 7 years ago

ill merge this for now and check the failing tests then :) thanks!

mcmar commented 7 years ago

@timse Can you cut a release with this included?

drwpow commented 7 years ago

I’m still getting the VipsJpeg warnings. My yarn.lock is the same:

srcset-loader@^2.2.0:
  version "2.2.0"
  resolved "https://registry.yarnpkg.com/srcset-loader/-/srcset-loader-2.2.0.tgz#998d2995641897c8039379cd15ae22ed7ddfeeba"
  dependencies:
    datauri "1.0.5"
    get-pixels "3.3.0"
    image-size "0.5.1"
    loader-utils "1.1.0"
    sharp "0.17.2"

Tried deleting yarn.lock, running yarn cache clear, etc. to no avail. Even tried deleting node_modules and re-installing, but still seeing the old version. Am I missing something?

dcalhoun commented 7 years ago

@dangodev pretty sure these changes have not been published in a release. This was merged 8 days ago. The last release was 12 days ago.

drwpow commented 7 years ago

🤦‍♂️ You’re right. I don’t know why I assumed this was published to NPM.

mcmar commented 7 years ago

This repo recently switched from prepublishOnly to prepare https://github.com/timse/srcset-loader/commit/81adaa01fd07dd6a9d4ee525d43c0d85de0c5012 That was supposed to make it so you could install straight from Github like this

"srcset-loader": "github:timse/srcset-loader",

But yarn doesn't like that, so we're still stuck waiting/begging for authors to cut releases to npm. @timse please please please. We're begging you.

timse commented 7 years ago

sorry again for the delay, it is/should be published as 2.2.1

something with the releases here is odd... sorry for that

drwpow commented 7 years ago

@timse No worries! I was facepalming myself, not you. You’re doing a great job!

I love this loader, BTW. It’s brilliant, and a game-changer for our team.

mcmar commented 7 years ago

@timse Somehow there was a mistake and v2.2.1 doesn't include this upgraded sharp. https://github.com/timse/srcset-loader/commit/03d3fc89879abdd18eb3a023b8533098a2bfc23a#diff-b9cfc7f2cdf78a7f4b91a753d10865a2R37

mcmar commented 7 years ago

It appears that there's a discrepancy between master versions and release versions. Here's 2.2.0 on master https://github.com/timse/srcset-loader/commit/62de8e138a37252da0686b0fc8b91e1ff69e102b And here's 2.2.0 on release https://github.com/timse/srcset-loader/commit/3d47948431ad2ffb8bd1d11ec52ffa4f67c925b4

Master 2.2.0 has sharp 0.18.2. Release 2.2.0 has sharp 0.17.2.

I'm not sure how this is happening. Maybe you're cutting releases from a different branch?

mcmar commented 7 years ago

In fact, 2.2.0 master did include the sharp upgrade. But the release 2.2.0 did not. So something is definitely odd with the releases.

timse commented 7 years ago

hum let me check - sorry for all that mess

timse commented 7 years ago

hum this is really odd - i released 2.2.2 the tags were in fact pointing to wrong commits - no idea why. i used npm version to create them... i fixed 2.2.1 but needed to patch bump to release a new version.

sorry for all the mess