Allow reading sandbox and sandboxGlobals from app's config/environment

[tomwayson]

This PR addresses this TODO:

ember-cli-fastboot/index.js

Lines 309 to 310 in 6336e97

	// TODO(future): make this configurable for allowing apps to pass sandboxGlobals
	// and custom sandbox class

So far the only way I've been able to get my addon to work is by passing sandboxGlobals to the FastBoot constructor (see ember-fastboot/fastboot#175).

The way I've achieved that is by adding a sandboxGlobals property to the fastboot hash in the app's config/environment:

// app's config/environment.js
module.exports = function(environment) {
  let ENV = {
    fastboot: {
      sandboxGlobals: {
        requireFunctionName: 'equireray'
      }
    }
  };
  return ENV;
};

Then w/ the changes in this PR I'm able to pass those along to FastBoot().

I only did it this way b/c it was the most expedient way to solve my problem, and I recognize that there might be a better way to pass these along. For this reason I stopped once I got it working and thought I'd check to see if you think I'm taking the best approach before investing in tests, adding docs to the README, etc.

I can confirm this does pass the sandboxGlobals param by running this from my app via npm link, but I did not try testing the sanbox parameter b/c I am not really sure of a valid use case for that. Also tests pass locally, but I did not add any tests b/c it was not obvious to me how I should test this feature.

Let me know if you think this is the right way to go about this, and if you'd like me to add tests to this PR, please point me in the right direction.

[kratiahuja]

Hi @tomwayson! Thanks for your PR! I like the overall idea and this was a TODO that I left around :). Sorry about that!

In my opinion, we shouldn't add the sandboxGlobals config in environment.js and instead pass it and build config from ember-cli-build.js if you only want this during development. However, I think we probably need a way to keep these variables in sync between and dev and production workflow. What about when using it with fastboot-app-server? How are you going to keep them in sync?

Regarding testing: I would just add a new route in the dummy app under test with passing the sandboxGlobals and accessing it. See the fastboot repo for example of a test.

[tomwayson]

instead pass it and build config from ember-cli-build.js

I was thinking the same. I could certainly update this PR to pass them that way, but:

I think we probably need a way to keep these variables in sync between and dev and production workflow. What about when using it with fastboot-app-server?

I hadn't thought about that, and haven't used fastboot-app-server yet so I'm not sure. Would it make more sense to add the option to FastBoot to be able to read those from a config file or node environment variables (instead of having ember-cli-fastboot and fastboot-app-server pass them in)?

[dmfenton]

@kratiahuja have you been able to consider @tomwayson's question?

[kratiahuja]

Would it make more sense to add the option to FastBoot to be able to read those from a config file or node environment variables (instead of having ember-cli-fastboot and fastboot-app-server pass them in)?

Sure we can do that but to me that is something that fastboot doesn't need to know. There is already other information that we pass as input so we should be consistent. I think we could have ember-cli-fastboot and fastboot-app-server read it from the same source of truth. Thoughts?

[tomwayson]

I think we could have ember-cli-fastboot and fastboot-app-server read it from the same source of truth.

That works for me. Any preference on that source of truth? config file? node environment variables? Is there already a precedent established for ember-cli-fastboot and fastboot-app-server reading common params?

[kratiahuja]

Let's support both and let's have node environment variable win. :)