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

Allow for simple replacement of variables that are part of other variables (like APP_SRC and DIST_DIR) #630

Closed
Paladinium opened this issue Mar 14, 2016 · 5 comments

Comments

@Paladinium
Copy link

The ProjectConfig allows to replace values from the extended SeedConfig. That works fine as long as the replaced variable is not as part of another variable. However, when it comes variables that are used as part of the definition of other variables, the replacement does not work as expected. For example:

export class ProjectConfig extends SeedConfig {
    constructor() {
        super();
        this.DIST_DIR = 'output'; // instead of 'dist'
        this.APP_SRC = 'app'; // instead of 'src'
    }
}

The variables for DIST_DIR and APP_SRC have properly been created. However, when running "npm start", you will notice that the CSS File has not been included in index.html and when running "npm run build.prod", the output gets partially created in 'output' and 'dist'. The variables partially use the values from ProjectConfig and also from SeedConfig.

This is because ProjectConfig needs to call SeedConfig's constructor before being able to set the variables. SeedConfig then initializes it's own variables as follows:

...
ASSETS_SRC           = `${this.APP_SRC}/assets`; // = src/assets
...
DEV_DEST             = `${this.DIST_DIR}/dev`; // = dist/dev
PROD_DEST            = `${this.DIST_DIR}/prod`;  // = dist/prod
TMP_DIR              = `${this.DIST_DIR}/tmp`;  // = dist/tmp
APP_DEST             = `${this.DIST_DIR}/${this.ENV}`;  // = dist/prod or dist/dev
...

Those variables use the original values of APP_SRC and DIST_DIR such that the overwrite of ProjectConfig is more or less useless. The workaround is the have ProjectConfig overwrite ALL those variables. Unfortunately, this is not very stable regarding updates of angular2-seed since one needs to watch out for newly introduced variables like the ones above.

@joshwiens
Copy link
Contributor

IMO it's a valid point though changing it would require a bit of planning to not cause existing projects a bit of pain.

//cc @ludohenin @mgechev @NathanWalker

@Paladinium
Copy link
Author

I totally understand, @d3viant0ne . Just to make the issue report complete, here is what needs to be changed when you want to change DIST_DIR properly:

- DIST_DIR
  |- DEV_DIST
  |- PROD_DIST
  |- TMP_DIR
     |-- SYSTEM_BUILDER_CONFIG
  |- APP_DEST
     |-- CSS_DEST
     |-- JS_DEST
     |-- APP_ROOT

APP_SRC only affects ASSETS_SRC and potentially APP_ASSETS (ProjectConfig currently overrides it which is why it would work).

@ludohenin
Copy link
Collaborator

@d3viant0ne see comment in #579

@mgechev
Copy link
Owner

mgechev commented Mar 17, 2016

@Paladinium makes sense. This is part of the roadmap in #579 so we can close this particular issue.

@Paladinium
Copy link
Author

Fine for me, @mgechev .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants