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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

{{#if-flags-}} dont always reflect the featureFlags value #21

Closed
jobackman opened this issue Mar 9, 2016 · 5 comments 路 Fixed by #25
Closed

{{#if-flags-}} dont always reflect the featureFlags value #21

jobackman opened this issue Mar 9, 2016 · 5 comments 路 Fixed by #25

Comments

@jobackman
Copy link
Contributor

Hi, before I begin I'd like to commend you on a great addon, it's just what I've been wanting for our project! 馃憤馃憤

I do believe I have found an issue in the ``{{if-flag-ENABLE_FOO}}helper though, wherefeatureFlags.keys` will not update the `if-flags` correctly. I tried reverse engineering it and I can continuously recreate the it now.

I made a fresh dummy app to make sure it wasn't our app causing the problems, and in case anyone wants to give it a try.

How to recreate:

// environment.js

// ...

featureFlags: {
  ENABLE_FOO: false,
  ENABLE_BAR: false
},
includeDirByFlag: {
  ENABLE_FOO: [/pods\/foo/],
  ENABLE_BAR: [/pods\/bar/]
}

// ...

if (process.env.baz === 'foo') {
  ENV.featureFlags.ENABLE_FOO = true;
}

if (process.env.baz === 'bar') {
  ENV.featureFlags.ENABLE_BAR = true;
}
<h2 id="title">Welcome to Ember</h2>

{{#if-flag-ENABLE_FOO}}
  <h1>FOO</h1>
{{/if-flag-ENABLE_FOO}}

{{#if-flag-ENABLE_BAR}}
  <h1>BAR</h1>
{{/if-flag-ENABLE_BAR}}

{{outlet}}
  1. Run the app; ember serve and neither <h1>foo</h1> nor <h1>bar</h1> will show in the template and their pods aren't included in the build. Nothing wrong here.
  2. Now restart the server, without changing any code, but with setting baz; baz=foo ember serve, thus enabling ENABLE_FOO. The app should now include the foo pod, which it does. But <h1>foo</h1> will still not show up in the template.
  3. If you want foo to show in the template now, edit something in your project and save, causing live-reload to recompile and create a fresh build of your app. Now only <h1>foo</h1> shows and is included in the build as it should.

I'm far from an expert on how you made you app how it should be made for that matter, but I noticed that the console.log spam "sdf" and "foo" (which btw are present in the npm package but not your repo) only occur when changes are made to a file in the app and saved, and thus recompiled in a new build.

I've tried removing dist and tmp, thinking caching of the build was the issue, but the end result has remained the same as long as I didn't edit files on the app.

I hope it makes sense!

@jobackman jobackman changed the title {{#if-flags-}} dont always reflect the featureFlags value {{#if-flags-}} dont always reflect the featureFlags value Mar 9, 2016
@minichate
Copy link
Owner

Hey @jobackman!

Unfortunately its a known issue that I/we've been fighting with for the last couple months. What we currently do is rm -rf node_modules; npm install when changing feature flags. Not ideal, I know.

HTMLBars heavily caches the AST after transformations are applied. We're still working on bypassing that cache, but in the meantime the best thing to do is clear out node_modules when changing code on either side of the flag.

I'll leave this issue open to track progress on the issue.

Thanks for reporting!

@oscarni
Copy link
Contributor

oscarni commented Apr 14, 2016

馃憤 Would be awesome if you could release an updated version to npm without console.log("sdf") and console.log("foo"). They fill up the therminal when developing :/

Otherwise very nice addon! Thanks

@minichate
Copy link
Owner

@oscarni Can you try 0.4.11 and see if that works for you?

@oscarni
Copy link
Contributor

oscarni commented Apr 18, 2016

@minichate works like a charm! Thanks for making the console clean 馃憤

@minichate
Copy link
Owner

Shipped in 0.4.13. Thanks so much @oscarni!

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 a pull request may close this issue.

4 participants
@minichate @jobackman @oscarni and others