Topic feature flag env var #6206
Conversation
Mostly copied that structure and have the parsing of terminal environment variable working. Need bridge the addon snippet that populates the flag variables for runtime consumption.
| ENV[flag] = EmberENV[flag] === true; | ||
| } | ||
| } | ||
| })(global.EmberENV || global.ENV); |
There was a problem hiding this comment.
We should use EmberDataENV or something (not the same global that Ember uses).
There was a problem hiding this comment.
I'll get this and the remaining feedback addressed tomorrow.
| // export const sample_feature_flag = true; | ||
| import { assign } from '@ember/polyfills'; | ||
| import global from '@ember-data/canary-features/environment'; | ||
| // import { ENV } from '@ember-data/canary-features/ |
| @@ -1 +1,32 @@ | |||
| // export const sample_feature_flag = true; | |||
| })(global.EmberENV || global.ENV); | ||
|
|
||
| export const DEFAULT_FEATURES = { | ||
| sample_feature_flag: null, |
There was a problem hiding this comment.
I vaguely prefer the SCREAMING_SNAKE_CASE here, though I know this isn’t really new (or the point of the PR)
There was a problem hiding this comment.
Given Ember uses that same convention, I agree to make it the same. I don't see warrant for introducing a new convention.
| sample_feature_flag: null, | ||
| }; | ||
|
|
||
| export const FEATURES = assign(DEFAULT_FEATURES, ENV.FEATURES); |
There was a problem hiding this comment.
You don’t want to mutate DEFAULT_FEATURES here, probably want assign({}, DEFAULT_FEATURES, ENV.FEATURES)
| }; | ||
|
|
||
| export const FEATURES = assign(DEFAULT_FEATURES, ENV.FEATURES); | ||
| export const sample_feature_flag = DEFAULT_FEATURES.sample_feature_flag; |
There was a problem hiding this comment.
Should be = FEATURES.sample_feature_flag
|
Addressed most of the feedback, but this still does not work when testing enabling a flag within an ember data file. I tested with the following in internal model's constructor: import { SAMPLE_FEATURE_FLAG } from '@ember-data/canary-features';
// ...
if (SAMPLE_FEATURE_FLAG) {
console.log('hi')
}I think I'm missing how exactly debug macros correctly serializes the environment / variables from features.js into a module the browser can understand. |
|
@efx I tested it and it actually worked for me |
* mimic @ember/canary-features Mostly copied that structure and have the parsing of terminal environment variable working. Need bridge the addon snippet that populates the flag variables for runtime consumption.
* mimic @ember/canary-features Mostly copied that structure and have the parsing of terminal environment variable working. Need bridge the addon snippet that populates the flag variables for runtime consumption.
|
FYI - Igor and I did some more work on the feature flagging setup over in #6197. AFAICT the main thing left is parsing out the default features and values from |
* mimic @ember/canary-features Mostly copied that structure and have the parsing of terminal environment variable working. Need bridge the addon snippet that populates the flag variables for runtime consumption.
* mimic @ember/canary-features Mostly copied that structure and have the parsing of terminal environment variable working. Need bridge the addon snippet that populates the flag variables for runtime consumption.
No description provided.