fix: enable serving in separate process from the build #332
Conversation
|
test failures are unrelated, tests appear to be broken on master |
| @@ -124,10 +130,13 @@ module.exports = { | |||
| // Use policy for test environment if both of these conditions are met: | |||
| // 1. the request is for tests and | |||
| // 2. the build include tests | |||
| let buildIncludeTests = this.app.tests; | |||
| const env = process.env.EMBER_ENV; | |||
| let buildIncludeTests = this.app | |||
There was a problem hiding this comment.
I'm wondering if we actually need to determine if build includes tests. Maybe it's enough to analyze if the request is for tests? Is there a case of legitim requests for /tests if app does not include tests?
There was a problem hiding this comment.
- you can do a build and then serve tests off the build directly.
- you can name a route tests :(
| @@ -101,6 +104,9 @@ module.exports = { | |||
| }, | |||
|
|
|||
| serverMiddleware: function ({ app: expressApp, options }) { | |||
| if (process.env.DISABLE_CSP) { | |||
There was a problem hiding this comment.
Any reason to have an environment variable to disable CSP in addon hard-coded? Having it in userland configuration would reduce complexity. A consumer would not need to look into addons documentation but only in project's CSP configuration to discover and understand the environment variable.
There was a problem hiding this comment.
this allows libraries to easily turn it off via config when running the command vs needing to learn about and change config files
| @@ -67,6 +67,9 @@ module.exports = { | |||
| // FastBoot. This one is returned here as default configuration in order to make it | |||
| // available at run time. | |||
| config: function (environment, runConfig) { | |||
| if (process.env.DISABLE_CSP) { | |||
There was a problem hiding this comment.
I had to return after setting _runConfig, because https://github.com/rwjblue/ember-cli-content-security-policy/blob/master/lib/utils.js#L96 was null (Cannot read properties of null (reading 'contentSecurityPolicy'))
this._runConfig = runConfig;
if (process.env.DISABLE_CSP) {
return {};
}
|
@runspired thanks a lot for this fix! Any chance of merge it? |
resolves #331
If serving from a separate command and your build includes tests you will also want to set the env var
USE_TESTS_CSP=1.Optionally if necessary the CSP can be disabled with
DISABLE_CSPenv var.