cujojs / when

A solid, fast Promises/A+ and when() implementation, plus other async goodies.
Other
3.44k stars 396 forks source link

Promise.js first function never runs #503

Closed NorthDecoder closed 6 years ago

NorthDecoder commented 6 years ago

How often can you reproduce it?

Description: The very first function in the first line of when/es6-shim/Promise.js is never called because it is missing ending parenthesis.

!function(e){
    "object"==typeof exports?module.exports=e():"function"==typeof define&&define.amd?define(e):"undefined"!=typeof window?window.Promise=e():"undefined"!=typeof global?global.Promise=e():"undefined"!=typeof self&&(self.Promise=e())
} //() adding parenthesis here will cause it to be called

// if it does run when parenthesis are added it will fail with error
//   TypeError: e is not a function

A simple test is to paste the function into the Node or the browser REPL.

A simple test scenario looks like this:

function testTheCallback (cb) { console.log( cb() ); }

function e () {return "callback e";}
console.log("1. " + e() +" is a " + typeof(e) ); // callback e is a function

!function(e){console.log( "this function is NEVER called!"  )} // is never called
!function(e){
  console.log( "2. second function is called, argument `e` is " + typeof(e) )
  }() // parenthesis cause call, but callback e is undefined...

// here in a named function the callback works
console.log("3. testTheCallback(): ");
testTheCallback( e ); // logs the string that e returns `callback e`

Steps to reproduce:

  1. Link to test jsfiddle; see the browser console for the logged results.
  2. Detail the exact steps taken to produce the problem a. Git clone When version 3.7.8 b. In Promise.js file added carriage return between first and second function c. Run npm test on command line, see no failures. d. Comment out the first function, rerun the tests, see no failures. e. Remove comment, added parenthesis after the function rerun the tests, see no failures. f. It appears this function is irrelevant. g. Copy and paste the function into the Node REPL, it returns false. h. Copy and paste the function into the browser REPL, it returns false. i. Run !function(e){} in REPL and it returns false too! j. It appears this function is irrelevant.

Expected results: Expect that the function would be needed by the script and run at some point. My conclusion is that this function is unnecessary.

Actual results: Running the function fails as expected, TypeError: e is not a function apparently due to passing function e into an anonymous function.

Environment:

Software Version
When version 3.7.8
es6-shim/Promise.js na
node -v 6.11.2
npm -v 3.10.10
Browser Firefox 58.0.1 (64-bit)
Operating system x86_64 GNU/Linux
briancavalier commented 6 years ago

@NorthDecoder thanks for the report.

Promise.js is a boilerplate wrapper generated by the browserify build that polyfills the Promise global. I verified it in Node by requiring Promise.js and seeing that it does overwrite the Promise global as intended.

See below--I inserted line breaks below for readability. You can see that inspecting the global Promise initially shows Node's builtin Promise function. Then, after requiring Promise.js, when's Promise polyfill is printed twice: the first is because require returns the Promise and the REPL prints it, and the second is my inspecting the global Promise a second time to show that it has been overwritten.

Are you seeing a case where requiring the file doesn't install the new Promise global? If so, rather than attempting to decipher browserify's boilerplate, a procedure for reproducing the case where the file is required and results in the global Promise not being overwritten would be the most helpful next step.

Perhaps you could try the same steps as below and see if that reveals anything interesting.


❯ cd /tmp

/private/tmp
❯ mkdir when-test

/private/tmp
❯ cd when-test

/private/tmp/when-test
❯ yarn add when
yarn add v1.3.2
info No lockfile found.
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
[3/4] 🔗  Linking dependencies...
[4/4] 📃  Building fresh packages...
success Saved lockfile.
success Saved 1 new dependency.
└─ when@3.7.8
✨  Done in 0.62s.

/private/tmp/when-test
❯ fnm use 6.11.2

/private/tmp/when-test
❯ node -v
v6.11.2

/private/tmp/when-test
❯ node
> Promise
[Function: Promise]

> require('when/es6-shim/Promise')
{ [Function: Promise]
  resolve: [Function: resolve],
  reject: [Function: reject],
  never: [Function: never],
  _defer: [Function: defer],
  _handler: [Function: getHandler],
  all: [Function: all],
  race: [Function: race],
  _traverse: [Function: traverse],
  _visitRemaining: [Function: visitRemaining],
  onFatalRejection: [Function],
  onPotentiallyUnhandledRejectionHandled: [Function],
  onPotentiallyUnhandledRejection: [Function],
  exitContext: [Function: noop],
  enterContext: [Function: noop],
  createContext: [Function: noop] }

> Promise
{ [Function: Promise]
  resolve: [Function: resolve],
  reject: [Function: reject],
  never: [Function: never],
  _defer: [Function: defer],
  _handler: [Function: getHandler],
  all: [Function: all],
  race: [Function: race],
  _traverse: [Function: traverse],
  _visitRemaining: [Function: visitRemaining],
  onFatalRejection: [Function],
  onPotentiallyUnhandledRejectionHandled: [Function],
  onPotentiallyUnhandledRejection: [Function],
  exitContext: [Function: noop],
  enterContext: [Function: noop],
  createContext: [Function: noop] }
NorthDecoder commented 6 years ago

Thank you for the response. I started out trying to understand the applicability of Stealjs by writing some tests of my own, proceeding by reading each line of code and adding a test for it. When asked the same question as above, the maintainers on the Stealjs project pointed out the anonymous function was included from the when/es6-shim/Promise.js project with a build tool. Therefore I proposed the same question here.

Since all tests are passing and there is no 'failure mode', I think I understand the answer to be: do not try to understand the output of a build tool, if it works just use it. It is a question of code coverage in test, however since this exercise is just for my edification, not fixing a problem I should close the issue and be :smile: