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

Check if environment is production-like in stripped-build-plugins #5234

Merged
merged 1 commit into from
Oct 23, 2017

Conversation

IceDragon200
Copy link
Contributor

On request, a PR, to fix the production like environment issue

Related

#5233

@rwjblue
Copy link
Member

rwjblue commented Oct 18, 2017

Seems good for us to be consistent here, but there are only three possible values for process.env.EMBER_ENV that are valid: test, development, and production.

I'd prefer to change the index.js check to be === 'production' (instead of changing the other to use a regexp)...

@runspired
Copy link
Contributor

runspired commented Oct 18, 2017

@rwjblue I agree that ember-cli doesn't support it in theory but I also know that historically we've loosely matched production and many teams have relied on that for configuring production-like staging environments (often began before ember-cli-deploy paved the path for nice things).

That said, changing to === here would also fix the issue that @IceDragon200 encountered, so I'm not super committed to keeping it loose.

@IceDragon200
Copy link
Contributor Author

@rwjblue, @runspired Well prior to updating to 2.16 (coming from 2.9) it was working fine with the production-like environments, it kinda threw a wrench in my day after the upgrade.

Maybe keeping it loose for now but adding a deprecation warning if a production-like environment is found:

if (env !== 'production' && /production/.test(env)) {
 warn("production like environments are deprecated, if using ember-cli-deploy, please configure your build using 'production'");
}

This would ensure folks like me, who expected the old behaviour are given fair warning and a path for migration before completely flipping the table on them.

Sorry about the late reply, it was a tricky week

@bmac
Copy link
Member

bmac commented Oct 23, 2017

@IceDragon200 that seems like a reasonable compromise. Do you need this change back-ported into 2.16?

@bmac bmac merged commit 2ba40b9 into emberjs:master Oct 23, 2017
@IceDragon200
Copy link
Contributor Author

@bmac Well I've already bitten the bullet and changed my code to use environment variables, so, not for me, but for anyone else that encounters this problem.

tl;dr Sure go ahead :)

@runspired
Copy link
Contributor

@IceDragon200 fwiw what I was saying was that as long as both checks match this should resolve your issue. Granted === means you won't get the code stripping in production-like environments, but it won't error either. I agree with @bmac that a deprecation for production-like is reasonable.

@bmac
Copy link
Member

bmac commented Oct 23, 2017

I added a pr for introducing the production like warning. #5239

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.

4 participants