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

Use FastBoot transform instead of phoenix-stub #80

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jeffjewiss
Copy link

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.

@bcardarella
Copy link
Collaborator

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.

@jeffjewiss
Copy link
Author

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

@mcfiredrill
Copy link

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.

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

Successfully merging this pull request may close these issues.

3 participants