Skip to content
This repository has been archived by the owner on Sep 11, 2018. It is now read-only.

Implement application configuration base on build environment #160

Closed
sultaniman opened this issue Nov 4, 2015 · 6 comments
Closed

Implement application configuration base on build environment #160

sultaniman opened this issue Nov 4, 2015 · 6 comments
Labels

Comments

@sultaniman
Copy link

Hi,

I will be very happy to implement this improvement so users can use environment based application (in browser) configuration

import config from 'app-config';

Possible points for implementation need to modify https://github.com/davezuko/react-redux-starter-kit/blob/master/config/index.js and change this

  1. Create a folder with application configuration in config directory,
  2. Add config.set('app_config', 'config/application');,
  3. Add app_config : project.bind(null, config.get('app_config')) to paths

Change this

config.set('utils_aliases', [
  'actions',
  'components',
  'constants',
  'containers',
  'layouts',
  'reducers',
  'routes',
  'services',
  'styles',
  'utils',
  'views'
].reduce((acc, dir) => ((acc[dir] = paths.src(dir)) && acc), {}));

to

config.set('utils_aliases',
  [
    'actions',
    'components',
    'constants',
    'containers',
    'layouts',
    'reducers',
    'routes',
    'services',
    'styles',
    'utils',
    'views',
    'config'
  ].reduce(
    (acc, x) => {
      if (x === 'config') {
        return ((acc[x] = paths.app_config(env)) && acc);
      }

      return ((acc[x] = paths.src(x)) && acc);
    },  {}
  )
);

Or to

let module_alases = [
    'actions',
    'components',
    'constants',
    'containers',
    'layouts',
    'reducers',
    'routes',
    'services',
    'styles',
    'utils',
    'views'
].reduce((acc, dir) => ((acc[dir] = paths.src(dir)) && acc), {}));

module_alases['app-config'] = paths.app_config(dir);

config.set('utils_aliases', module_alases);

What do you think please advise.

@dvdzkwsk
Copy link
Owner

dvdzkwsk commented Nov 5, 2015

Interesting idea @imanhodjaev, but what about just adding a config alias and then allowing people to just do:

import config from 'config'

Where the file is ~/src/config.js or ~/src/config/index.js. I understand where you're coming from with the fact that the app config lives inside of ~/config, but that config is specifically for the project and build system, and as a result I think it would be confusing to have application code sitting outside of ~/src.

@sultaniman
Copy link
Author

@davezuko sure we can keep application configuration inside src directory.
I am agree with you that keeping configuration outside of src will confuse people.
The main idea of this proposal was to be able to separate configuration while developing and deploying production ready configuration relying on NODE_ENV=development | production | etc.

src
   config
         development.js
         production.js

Edited: 6 Nov. 2015 15:50
Also I think when you're in for example development environment you want webpack to automatically take config/development (for production config/production) and import it as

import config from 'config'

instead of checking if

flag=development | production

@dvdzkwsk
Copy link
Owner

dvdzkwsk commented Nov 8, 2015

@imanhodjaev I totally see where you're coming from and agree that this would be useful, but I just don't think it's something that needs to be handled by the starter kit. As you've shown it's not too complicated to add this functionality, and somebody needing this sort of configuration can probably figure it out fairly quickly. That said, I wouldn't be opposed to adding more documentation to the README to demonstrate how to incorporate functionality such as this.

@sultaniman
Copy link
Author

@davezuko so once we know it can be implemented easily can I make a pull request with a small guide in the README?

@dvdzkwsk
Copy link
Owner

@imanhodjaev if you want to take a shot at documenting it in the README go for it. I can't say for sure that I'll merge it in, but definitely willing to consider it.

@dvdzkwsk
Copy link
Owner

dvdzkwsk commented Dec 5, 2015

Closing this for now, if you end up submitting a PR I'd be happy to review it. Thanks!

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

No branches or pull requests

2 participants