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

pify for gm doesn't seems to be working #36

Closed
kdybicz opened this issue Jan 29, 2017 · 12 comments
Closed

pify for gm doesn't seems to be working #36

kdybicz opened this issue Jan 29, 2017 · 12 comments
Assignees

Comments

@kdybicz
Copy link

kdybicz commented Jan 29, 2017

Hi,

I'm trying to fix AVA tests for some functionality and I need to Promisify gm(...).size() function. Unfortunately I'm not able to do that properly:

const out = await pify(gm(image.data).size());
console.log("gm output: " + out.width);
---
  gm output: undefined
const out = await pify(gm(image.data).size)();
console.log("gm output: " + out.width);
---
  1 failed

  resize-jpeg › Resize JPEG with jpegoptim
  Error: Cannot read property 'size' of undefined
    proto.(anonymous function) (node_modules/gm/lib/getters.js:47:20)
    node_modules/pify/index.js:29:7
    node_modules/pify/index.js:12:10
    ret (node_modules/pify/index.js:56:34)
    Test.<anonymous> (test/resize-jpeg.js:37:20)
    Generator.next (<anonymous>)
const out = await pify(gm(image.data).size.bind(gm))();
console.log("gm output: " + out.width);
---
  1 failed

  resize-jpeg › Resize JPEG with jpegoptim
  Error: Cannot read property 'size' of undefined
    proto.(anonymous function) (node_modules/gm/lib/getters.js:47:20)
    node_modules/pify/index.js:29:7
    node_modules/pify/index.js:12:10
    ret (node_modules/pify/index.js:56:34)
    Test.<anonymous> (test/resize-jpeg.js:37:20)
    Generator.next (<anonymous>)
const out = await pify(gm(image.data)).size();
console.log("gm output: " + out.width);
---
  1 failed

  resize-jpeg › Resize JPEG with jpegoptim
  Error: pify(...).size is not a function
    Test.<anonymous> (test/resize-jpeg.js:37:43)
    Generator.next (<anonymous>)

So, my question is: if it's possible to pify gm and if so, I would like to know how?

@SamVerschueren
Copy link
Contributor

SamVerschueren commented Jan 29, 2017

Try this

const image = gm();

pify(image.bind(image));

@kdybicz
Copy link
Author

kdybicz commented Jan 29, 2017

Doesn't work:

  1 failed

  resize-jpeg › Resize JPEG with jpegoptim
  Error: image.bind is not a function
    Test.<anonymous> (test/resize-jpeg.js:39:32)
    Generator.next (<anonymous>)

neither does Example 3.

@SamVerschueren
Copy link
Contributor

SamVerschueren commented Jan 29, 2017

Oh yes off course. You have to pify pity the write function, not the others.

const image = gm().size(100, 100);

pify(image.write.bind(image))().then(...);

@schnittstabil
Copy link
Contributor

schnittstabil commented Jan 29, 2017

Your first and your second outputs show, that gm(image.data) returns undefined:

const out = await pify(gm(image.data).size)();
const out = await pify(gm(image.data).size.bind(gm))();
---
  Error: Cannot read property 'size' of undefined

At least these issues seem to be unrelated to pify.

Aww, sorry, #16 seems related to your issue.


About your third example:

const out = await pify(gm(image.data)).size();
---
  Error: pify(...).size is not a function

I cannot guess what size is: a number or a string or …? – @kdybicz Do you mind investigating the type and value of size?

@kdybicz
Copy link
Author

kdybicz commented Jan 29, 2017

size is a function http://aheckmann.github.io/gm/docs.html#getters

gm(image.data) returns undefined

This one occures only when using pify().

console.log("Plain gm: " + gm(image.data));
console.log("Type of gm.size: " + typeof gm(image.data).size);
const out = await pify(gm(image.data).size)();
---
Plain gm: [object Object]
Type of gm.size: function

  1 failed

  resize-jpeg › Resize JPEG with jpegoptim
  Error: Cannot read property 'size' of undefined
    proto.(anonymous function) (node_modules/gm/lib/getters.js:47:20)
    node_modules/pify/index.js:29:7
    node_modules/pify/index.js:12:10
    ret (node_modules/pify/index.js:56:34)
    Test.<anonymous> (test/resize-jpeg.js:40:20)
    Generator.next (<anonymous>)

Edit: yeah, I was afraid that this could be related to #16

@schnittstabil
Copy link
Contributor

schnittstabil commented Jan 29, 2017

Maybe you want to try this:

const img = gm(image.data);
const out = await pify(img.size.bind(img))(); // <-- bind to img, not to gm!

If it works you may want to try bind-methods auto-bind as long as #16 isn't solved:

// untested(!)
const autoBind = require('auto-bind');
const gm = require("gm").subClass({ imageMagick: true });
const gmP = (...args) => pify(autoBind(gm(...args)));

test("Resize JPEG with jpegoptim", async t => {
    // …
    const out = await gmP(destPath).size();
    t.is(out.width, 200);
});

@kdybicz
Copy link
Author

kdybicz commented Jan 29, 2017

Unfortunately this doesn't seems to be working, I'm getting:

  1 failed

  resize-jpeg › Resize JPEG with jpegoptim
  Error: Cannot convert undefined or null to object
    Function.keys (<anonymous>)
    module.exports (node_modules/pify/index.js:59:16)
    gmP (test/resize-jpeg.js:11:26)
    Test.<anonymous> (test/resize-jpeg.js:39:20)
    Generator.next (<anonymous>)

@schnittstabil schnittstabil self-assigned this Jan 30, 2017
@schnittstabil
Copy link
Contributor

const img = gm(image.data);
const out = await pify(img.size.bind(img))(); // <-- bind to img, not to gm!

@kdybicz Above works as expected.

As a #16 workaround, I suggest a small helper function:

const bind = obj => {
    for (const key in obj) {
        const val = obj[key];

        if (typeof val === 'function') {
            obj[key] = val.bind(obj);
        }
    }

    return obj;
};

const gmP = (...args) => pify(bind(gm(...args)));

By the way, your tests have additional issues (see https://github.com/secretescapes/aws-lambda-image/pull/1):

@schnittstabil
Copy link
Contributor

I'm closing this, because it is a #16 duplicate.

@kdybicz
Copy link
Author

kdybicz commented Jan 30, 2017

This is my approach to solving this issue https://github.com/ysugimoto/aws-lambda-image/pull/87/files but I will for sure take a look at your proposal. Thanks!

@schnittstabil
Copy link
Contributor

Btw, bind-all provides the same functionality as my bind helper above:

const bindAll = require('bind-all');

const gmP = (...args) => pify(bindAll(gm(...args)));

test("Resize JPEG with cjpeg", async t => {
    //…
    const out = await gmP(destPath).size();
    t.is(out.width, 200);
});

@kdybicz
Copy link
Author

kdybicz commented Jan 30, 2017

Thanks to your advice I did end up with code, that is working perfectly fine! https://github.com/secretescapes/aws-lambda-image/blob/improving-tests-quality-v2/test/resize-jpeg.js

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants