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

grunt 'build' copies the local env variable file (server/config/local.env.js) into /dist #1570

Closed
MagRelo opened this issue Jan 18, 2016 · 11 comments
Labels

Comments

@MagRelo
Copy link

MagRelo commented Jan 18, 2016

The grunt 'build' task copies the local env variable file (server/config/local.env.js) into /dist – is this intentional? I've always modified my gruntfile to exclude this file from the 'copy' config, but that doesn't seem to work for me anymore with the babel:server task. I haven't been able to find the right syntax to exclude this file from the babel task...

Q1) Should the local ENV vars be bundled into /dist with the grunt build task?
Q2) If so, could someone help me with the syntax to exclude them on my end?

Thanks, great project!

@Awk34 Awk34 added the bug label Jan 22, 2016
Awk34 added a commit that referenced this issue Jan 28, 2016
@MagRelo
Copy link
Author

MagRelo commented Feb 2, 2016

Hi, I appreciate the quick turn-around, but this doesn't seem to work for me (maybe I'm taking crazy pills...)

I updated to 3.3.0 and created a new project (yo-rc.json here) and both local.env.sample.js and local.env.js are both copied into the /dist. (btw, I think local.env.js should also be excluded, yeah?)

Here is what my /dist looks like after running grunt build:
screen shot of dist directory

Am I crazy? : ) Is it just me?

@Awk34
Copy link
Member

Awk34 commented Feb 2, 2016

Do you have this line?

@MagRelo
Copy link
Author

MagRelo commented Feb 2, 2016

Yep, confirmed. As an aside, I think local.env.js should be added as well, yeah?

This is working for you though? maybe I have an out-of-date dep somewhere... (?)

@Awk34
Copy link
Member

Awk34 commented Feb 2, 2016

You know what, it's not getting copied anymore, but it's still probably getting put in /dist because of Babel. try adding '!config/local.env.sample.js' to the src array of babel.server.

As for your comment about local.env.js, look at the conversation in #1590. If you don't want it, exclude it yourself, but other's might want it.

@Awk34 Awk34 reopened this Feb 2, 2016
@MagRelo
Copy link
Author

MagRelo commented Feb 2, 2016

I added the line to babel but it doesn't seem to be working for me. I also tried adding <%= yeoman.server %> and a few variations but no luck. I think you're right that it's in babel but I haven't been able to find the right config to exclude it...

Per the discussion, it's awesome to see you've already considered it, you guys are awesome! It seems risky to me as a default, but, like you said, I can handle it on my side when I get the syntax figured out. Thanks for the support!

carragom pushed a commit to carragom/generator-angular-fullstack that referenced this issue Feb 9, 2016
Awk34 added a commit that referenced this issue Feb 19, 2016
@Awk34 Awk34 closed this as completed in 429d5f3 Mar 10, 2016
@aendra-rininsland
Copy link

@Awk34 I think this is still a problem, using Gulp + TypeScript. git secrets caught it the other day when I tried to deploy to Heroku.

Item Version
generator-angular-fullstack 3.5.0
Node 5.3.0
npm 3.6.0
Operating System OS X 10
Item Answer
Transpiler TypeScript
Markup HTML
CSS SCSS
Router ui-router
Build Tool Gulp
Client Tests Mocha
DB MongoDB
Auth Y -- Google

If you add the following to ln. 45 of gulpfile.babel.js (I.e., to path.server.scripts), it seems to solve the problem.

!${serverPath}/config/local.env.js

@Awk34
Copy link
Member

Awk34 commented Apr 21, 2016

@Aendrew we still copy local.env.js, just not local.env.sample.js

@aendra-rininsland
Copy link

@Awk34 Ah, I see. That sort of violates 12-factor ideas though, such that no instance ever has config outside of environment variables.

AFAICT, local.env.js is mainly used while developing locally; generally one would use actual env vars in production, and the presence of local.env.js might actually even confuse things as it'd override the instance env vars set during deployment (Correct me if I'm wrong or I'm misunderstanding something here). My suggestion would be to remove all local env-related files from dist, though happy to discuss the pros and cons of either approach.

@Koslun
Copy link
Member

Koslun commented Apr 25, 2016

@Aendrew I don't believe it breaks the 12-factor idea you linked. Their own litmus test is as follows:

A litmus test for whether an app has all config correctly factored out of the code is whether the codebase could be made open source at any moment, without compromising any credentials.

The local.env.js file is excluded from version control from the provided .gitignore rules and given that people follow the rule of creating a new local.env.js rather than changing the sample (local.env.sample.js) it is thus not part of version control which would compromise credentials upon sharing the code and break the stated litmus test.

A developer could possibly use the local.env.js file to simply load env variables rather than write them down outright. Where I figure that developing locally would see simply having it written down in a file as the easiest approach. But as stated I figure it should still be fine for production as well since it's still not being version controlled and does not part of your codebase, but if your deployment process does use environment variables you could simply load them here.

So I think it's fine the way it is, just load environment variables in local.env.js if you don't want any information stored in a file.

@Awk34
Copy link
Member

Awk34 commented Apr 25, 2016

@Aendrew I agree with @Koslun . I had originally excluded both, but reverted to only excluding the sample file. The regular local.env.js is excluded by git, so if you publish a repo with it, it will still only stay on your local copy. We copy it, though, in the case that you want to use the variables inside a dist environment locally.

@aendra-rininsland
Copy link

@Awk34 and @Koslun — Very fair points both. 😄

Many thanks for the explanation (and workaround, I'll just use env vars in local.env from here on in)!

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

No branches or pull requests

4 participants