facebookarchive / prepack

A JavaScript bundle optimizer.
http://prepack.io
Other
14.22k stars 424 forks source link

Support for promises #1306

Open jlongster opened 6 years ago

jlongster commented 6 years ago

Feel free to rename this issue if it's something more specific. This started in this comment: https://github.com/facebook/prepack/issues/1285#issuecomment-354828615 and I've found a reduced test case which I'm filing here.

According to Sebastian's comment in that issue, evaluating promises should work, but I can't get even a basic program working. The simplest program throws the error described in the other issue, "all native function values should be intrinsics". Here's the program:

https://prepack.io/repl.html#GYVwdgxgLglg9mABAdwIYygCjASkQbwChFEAnAUyhFKTHOUQAVS4BbGAZ3Mwo7gBsAbuUQBeAHyIuUACoxW5OCCy8BwgDSJcOANyEAvoUJoMmAIwAGKzgB0UABbkwmCAj79yN-nADmmAOT2MP44OEA

NTillmann commented 6 years ago

Indeed, Promises are not yet fully supported. There are certainly things missing in the serializer, but possibly more...

Here are some obvious things that need to be done:

  1. Search for "TODO #26" in the code. Cases for "Promise" need to be added.
  2. The new "Promise" cases will (hopefully) fail in a few places. Most notably in the ResidualHeapVisitor and ResidualHeapSerializer, where a case "Promise": needs to be added similar to case "Date":, as Sebastian said in https://github.com/facebook/prepack/issues/1285#issuecomment-354828615.
jlongster commented 6 years ago

Nice, thanks. I'd like to start contributing and that seems easy enough. Hopefully I'll find time soon (if someone else want to work on it please do).