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

Watch config/environment.js and export app-name/config module. #1777

Merged
merged 1 commit into from
Aug 27, 2014

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented Aug 27, 2014

  • Adds automatic reloads on config/environment.js changes.
  • Loads configuration info from a module (keeping backwards compat for MyAppNameENV).

Addresses #1750.

var outputPath = path.join(destDir, relativePath);
var configPath = path.join(this.options.project.root, srcDir, relativePath);
var contents = fs.readFileSync(configPath, {encoding: 'utf8'});
contents = contents.replace('module.exports = function(environment) {', '');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should just leave these, and then new Function in a 'use strict' closure that implements them.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@stefanpenner
Copy link
Contributor

sg

rwjblue added a commit that referenced this pull request Aug 27, 2014
Watch config/environment.js and export app-name/config module.
@rwjblue rwjblue merged commit c599852 into ember-cli:master Aug 27, 2014
@rwjblue rwjblue deleted the watch-config-env branch August 27, 2014 02:43
@inDream
Copy link
Contributor

inDream commented Aug 27, 2014

This is a breaking change and I think this should be warned.
I need to run ember init and overwrite index.html in order to make it works again.

@rwjblue
Copy link
Member Author

rwjblue commented Aug 27, 2014

Updated in 1c0c527 to indicate that it was a breaking change.

@stefanpenner
Copy link
Contributor

@inDream when upgrading you should likely always run ember init and walk through the changes. Hopefully soon this project will be stable, nothing will change and we can work on better then :P

@inDream
Copy link
Contributor

inDream commented Aug 27, 2014

@stefanpenner I've changed most files inside app/ so I seldom run ember init after updates.

@kanongil
Copy link
Contributor

I'm aware that this change is a breaking change, but did you consider environment.js files that require other files? I use this in my build system, and it is completely broken in 0.0.42.

@jakecraige
Copy link
Member

@kanongil Are you referring to using node's require?

I was doing something like that too. For me, it wasn't a big deal as it was a minor convenience, but it would be nice if it still worked. Seems like some cool things could be brought out of using node in that file

@kanongil
Copy link
Contributor

@jakecraige Exactly.

Another issue I just noticed is the interaction with the environment and getEnvJSON EmberApp options. As far as I can tell these options are effectively broken now.

@rwjblue
Copy link
Member Author

rwjblue commented Aug 29, 2014

@jakecraige, @kanongil - I fixed the ability to have your config/environment.js file use require in #1809.

@rwjblue
Copy link
Member Author

rwjblue commented Aug 29, 2014

Also, when this transition is complete (still a few pieces to iron out), we will likely not use getEnvJSON at all.

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

Successfully merging this pull request may close these issues.

6 participants