Add environment config to builds

[filipesilva]

See #41 (comment)

Environments

At build time, the src/client/app/environment.ts will be replaced by either
config/environment.dev.ts or config/environment.prod.ts, depending on the
current cli environment.

Environment defaults to dev, but you can generate a production build via
the -prod flag in either ng build -prod or ng serve -prod.

[hansl]

This LGTM, let's see what comes out of the discussion in styleguide.

[hansl]

So after working with Igor and Filipe during the meeting, here's the path forward:

1. Create 3 JSON files in the scaffold; 1 default environment template, and 2 environments that will be applied on top of the default one. Let the user add more if he wishes to. These JSON files will be used by the build step. They could be part of the configuration (angular-cli.json), I'm open to either that or separate files somewhere.
2. Change the index.html file in the scaffold to include a line like <script>__ENV__ = {{ENV_JSON}};</script> in the headers.
3. The environment.ts file contains

declare var __ENV__ : { [name: string]: any };
export default __ENV__;

1. At build, takes the environment name, get the corresponding JSON file, apply on top of the default, insert it into the index.html.
2. Add to the app.ts:

import environment from 'environment';
// ...
if (environment.production) { enableProdMode(); }

1. Anyone that wants to have environment variable can simply import environment from 'environment';.

[hansl]

What do you think?

[filipesilva]

I like the simpler enableProdMode(); check in app.ts, it looks cleaner. In retrospective I'm not really sure why I made it a provider initially...

I guess it caters to the server-side rendering scenario by moving the logic elsewhere, so the bootstrap doesn't have to be more complex. Now that I think of it, on apps with a lot of nested routes I'd much prefer to have a provider I can use than to do import environment from '../../../../../../../../environment'. So in those two cases a provider is better imho.

What's the usecase for having a default template? Is this something you've felt the need for? I haven't yet, but that doesn't mean it's not useful.

I'm not too keen on putting stuff on index.html myself, as I think it's a bit roundabout and not a very clean way of doing it...

In this scenario, we create json config files and then use the build system to apply them to index.html via a global var, so that we have access to them inside the application. I'd much rather instead to have ts files exporting the same thing and just overwrite environment.ts on the build system.

End result would be the same, without having to fiddle with index.html and with the added benefit of code intel without needing anything else. Logic on the build system would also be similar, read from a file and write to another, but instead of adding code in the middle of a file we'd replace it entirely.

[hansl]

So in Django you get a default set of options for the environment, which is overwritten by the environment that you need.

So something like this exists:

// environment.default.ts
export default = {
  production: false,
  backendUrl: 'http://localhost:9999/',
  firebaseUrl: 'http://blah/',
};

// environment.dev.ts
import DEFAULTS from './environment.default'
export default = Object.assign(DEFAULTS, {
});

// environment.e2e.ts
import DEFAULTS from './environment.default'
export default = Object.assign(DEFAULTS, {
  production: true,
  // Use same backends as DEV
});

// environment.prod.ts
import DEFAULTS from './environment.default'
export default = Object.assign(DEFAULTS, {
  production: true,
  backendUrl: 'http://blah',
  firebaseUrl: 'http://bleh/',
});

I guess the default could always be DEV, but it's better to dissociate DEV from anything else since it might lead to mistakes and expose data you don't want to.

[filipesilva]

Thinking a bit more about enableProdMode(), I think it makes more sense for it to be in app.ts because it's directly bootstrap related.

That way the provider can be a straight up import of environment.ts.

[filipesilva]

Ok the defaults make more sense now. These env files seem the perfect place to add the firebase info, and even APP_BASE_HREF (github deploys need a specific one, for instance).

[filipesilva]

Removed provider, but didn't incorporate the defaults because it made the process more complicated when using more environments that don't extend dev (local for instance), and using APP_BASE_HREF isn't feasible since SystemJS still needs the base tag in order to correctly load files.

[hansl]

We already have a config/ directory, why not reuse it? :)

[filipesilva]

I thought about it at the start, but then assumed it would be problematic because of the whole client dir thing since we could have more than one app.

But with you mentioning it now I agree it's much better to have those files out of the client and in config.

[hansl]

Split the lines, this might trigger the linter :)

[filipesilva]

Roger.

[hansl]

Can you add the overwriting of the environment by the build system in this PR? Then I think this will be ready to go.

[filipesilva]

@hansl done, please review.

[filipesilva]

@hansl tests are fixed.

[hansl]

LGTM. We should add support for multiple builds in the future, but for now this is 👍.

[filipesilva]

@hansl agree, atm just wanted to get something in this week.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}