Skip to content

Promisify breaks resolve #151

Closed
Closed

Description

@reggi

I have an example of tests passing (can't publish at the moment) with require.resolve and with resolve as a callback, but when I wrap resolve in const resolveAsync = Promise.promisify(resolve) for some reason the tests that worked are failing. I've also implemented my own wrapped promise and some of tests pass, and others fail, specifically cases where there's a package.json resolve.

Any ideas why this package is un-promisify-able, and potential ways around it?

Activity

  1. ljharb commented on Feb 22, 2018

    @ljharb
    Member

    What promisify implementation are you using?

    Since https://npmjs.com/util.promisify is built into node, that’s really the only implementation that matters.

    If resolve can’t promisify by default, we could maybe add a custom promisify symbol.

    Does it work with the canonical implementation?

  2. reggi commented on Feb 22, 2018

    @reggi
    Author

    I've been using Bluebird to promisify, but even an initial check here.

    ➜  ~ trymodule util.promisify resolve
    Gonna start a REPL with packages installed and loaded for you
    'util.promisify' was already installed since before!
    'resolve' was already installed since before!
    Package 'util.promisify' was loaded and assigned to 'util_promisify' in the current scope
    Package 'resolve' was loaded and assigned to 'resolve' in the current scope
    REPL started...
    > require.resolve('./Desktop/jest-example/index.js')
    '/Users/treggi/Desktop/jest-example/index.js'
    > util.promisify(resolve)('./Desktop/jest-example/index.js').then(console.log)
    Promise {
      <pending>,
      domain:
       Domain {
         domain: null,
         _events: { error: [Function: debugDomainError] },
         _eventsCount: 1,
         _maxListeners: undefined,
         members: [] } }
    > (node:86567) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): Error: Cannot find module './Desktop/jest-example/index.js' from 'internal'
    (node:86567) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
    

    Running node 9.5

  3. ljharb commented on Feb 23, 2018

    @ljharb
    Member

    Unhandled promise rejection (rejection id: 1): Error: Cannot find module './Desktop/jest-example/index.js' from 'internal' - it seems like it's properly rejecting. Try adding .catch(e => console.error(e))?

  4. reggi commented on Feb 23, 2018

    @reggi
    Author
  5. ljharb commented on Feb 23, 2018

    @ljharb
    Member

    What happens with a non-promisified version of this module, however?

  6. reggi commented on Feb 23, 2018

    @reggi
    Author

    @ljharb It works perfectly fine

    Let me explain this example:

    ➜  ~ trymodule util.promisify resolve
    Gonna start a REPL with packages installed and loaded for you
    'util.promisify' was already installed since before!
    'resolve' was already installed since before!
    Package 'util.promisify' was loaded and assigned to 'util_promisify' in the current scope
    Package 'resolve' was loaded and assigned to 'resolve' in the current scope
    REPL started...

    Regular require.resolve works, and resolves this path.

    > require.resolve('./Desktop/jest-example/index.js')
    '/Users/treggi/Desktop/jest-example/index.js'
    

    The resolve module callback returns the correct resolved path.

    > resolve('./Desktop/jest-example/index.js', (err, value) => console.log(value))
    undefined
    > /Users/treggi/Desktop/jest-example/index.js
    

    Node 9's native util.promisify returns a promise, calls it's .catch and logs the error.

    > util.promisify(resolve)('./Desktop/jest-example/index.js').then(console.log).catch(e => console.log(e))
    Promise {
      <pending>,
      domain:
       Domain {
         domain: null,
         _events: { error: [Function: debugDomainError] },
         _eventsCount: 1,
         _maxListeners: undefined,
         members: [] } }
    > { Error: Cannot find module './Desktop/jest-example/index.js' from 'internal'
        at /Users/treggi/.trymodule/node_modules/resolve/lib/async.js:61:35
        at load (/Users/treggi/.trymodule/node_modules/resolve/lib/async.js:80:43)
        at onex (/Users/treggi/.trymodule/node_modules/resolve/lib/async.js:105:17)
        at /Users/treggi/.trymodule/node_modules/resolve/lib/async.js:26:73
        at FSReqWrap.oncomplete (fs.js:166:21) code: 'MODULE_NOT_FOUND' }
    

    The util.promisify module returns a promise, calls it's .catch and logs the error.

    > util_promisify(resolve)('./Desktop/jest-example/index.js').then(console.log).catch(e => console.log(e))
    Promise {
      <pending>,
      domain:
       Domain {
         domain: null,
         _events: { error: [Function: debugDomainError] },
         _eventsCount: 1,
         _maxListeners: undefined,
         members: [] } }
    > { Error: Cannot find module './Desktop/jest-example/index.js' from 'internal'
        at /Users/treggi/.trymodule/node_modules/resolve/lib/async.js:61:35
        at load (/Users/treggi/.trymodule/node_modules/resolve/lib/async.js:80:43)
        at onex (/Users/treggi/.trymodule/node_modules/resolve/lib/async.js:105:17)
        at /Users/treggi/.trymodule/node_modules/resolve/lib/async.js:26:73
        at FSReqWrap.oncomplete (fs.js:166:21) code: 'MODULE_NOT_FOUND' }
    
  7. reggi commented on Feb 24, 2018

    @reggi
    Author

    Any idea what's going on here @ljharb?

  8. ljharb commented on Feb 24, 2018

    @ljharb
    Member

    @reggi what happens if you call the promisified resolve with an empty object as the second argument?

  9. ljharb commented on Feb 24, 2018

    @ljharb
    Member

    After looking into this a bit more, the issue is that the assumption that "promisify will work on everything" is flawed; it won't.

    Using util.promisfy.custom, we could provide a custom implementation that would work with the native one (but not with bluebird) - but then this module would have to add a dependency on util.promisify and/or restrict which node versions it runs on.

    In other words, I think the proper solution is to wrap resolve in new Promise if you want to promisify it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions