ember-phoenix icon indicating copy to clipboard operation
ember-phoenix copied to clipboard

Use FastBoot transform instead of phoenix-stub

Open jeffjewiss opened this issue 7 years ago • 3 comments

I'm not entirely confident that this works as intended, but I followed the migration path followed by other add-ons. I wasn't sure if using the transform is preferable to the stub. I thought it would be easier to maintain if one could be removed though.

Unfortunately, I don't have a Phoenix project to test this with at the moment.

jeffjewiss avatar Jul 18 '17 11:07 jeffjewiss

I don't necessarily agree with this. It will put this addon on the path of disallowing fastboot for public API. There is the potential in the future where we can have some fastboot specific behavior.

bcardarella avatar Jul 18 '17 12:07 bcardarella

Okay, no problem.

Do you mind expanding on that a little, for my understanding?

The reason I'm asking is that it currently looks like the only different between phoenix.js and phoenix-stub.js is:

        if (window.XDomainRequest) {
          var req = new XDomainRequest(); // IE8, IE9
          this.xdomainRequest(req, method, endPoint, body, timeout, ontimeout, callback);
        } else {
          var req = window.XMLHttpRequest ? new XMLHttpRequest() : // IE7+, Firefox, Chrome, Opera, Safari
            new ActiveXObject("Microsoft.XMLHTTP"); // IE6, IE5
          this.xhrRequest(req, method, endPoint, accept, body, timeout, ontimeout, callback);
        }

So I figured it would be safe to use this since fastboot-transform should wrap this in a check for FastBoot.

Also, couldn't additional behaviour specific to FastBoot be added in the future with the new API?

updateFastBootManifest(manifest) {
  // you can decide on the order of control (whether to push or unshift)
  // once the build is done you will automically see `fastbootVendorLib` file under `dist/<addonname>` since the src library is in 
  // <addon>/public
  manifest.vendorFiles.push('assets/fastboot-lib.js');
  
  return manifest;
}

I'm cool with you closing this if you disagree with the change. I was hoping to learn a bit more about the ember–phoenix setup in the process of working on this.

Cheers

jeffjewiss avatar Jul 18 '17 13:07 jeffjewiss

Haven't tested this, but I would like something to stop phoenix from running in fastboot:

There was an error running your app in fastboot. More info about the error:                                                    
 ReferenceError: ActiveXObject is not defined   
    at Function.request (/Users/tony/src/datafruits/tmp/broccoli_merge_trees-output_path-ZPwO72G2.tmp/assets/vendor/phoenix.js:
1025:1)                                                                                                                        
    at LongPoll.poll (/Users/tony/src/datafruits/tmp/broccoli_merge_trees-output_path-ZPwO72G2.tmp/assets/vendor/phoenix.js:953
:1)                                                                                                                            
    at new LongPoll (/Users/tony/src/datafruits/tmp/broccoli_merge_trees-output_path-ZPwO72G2.tmp/assets/vendor/phoenix.js:919:
1)                                                                                                                             
    at Socket.connect (/Users/tony/src/datafruits/tmp/broccoli_merge_trees-output_path-ZPwO72G2.tmp/assets/vendor/phoenix.js:69
7:1)
    at Class.init (/Users/tony/src/datafruits/tmp/broccoli_merge_trees-output_path-ZPwO72G2.tmp/assets/datafruits13/services/ch
at.js:25:1)
    at new Class (/Users/tony/src/datafruits/tmp/broccoli_merge_trees-output_path-ZPwO72G2.tmp/assets/vendor/ember/ember.debug.
js:38511:1)
    at Function._ClassMixinProps.create (/Users/tony/src/datafruits/tmp/broccoli_merge_trees-output_path-ZPwO72G2.tmp/assets/ve
                                                                               

I'm not sure if there are any user cases for running phoenix in fastboot, if that is what @bcardarella is referring to.

mcfiredrill avatar Aug 22 '17 10:08 mcfiredrill