Fix usage within multi-EmberApp builds

[ef4]

It is possible to have more than one instanceof EmberApp in an ember-cli-build.js. In this case, an addon's included hook will run multiple times.

This is causing ember-cli-htmlbars-inline-precompile to register multiple times with babel, resulting in a build failure.

[pangratz]

Thanks for this PR, seems reasonable! The tests are unrelated to this change.

I think it would be nice if this could be tested, not sure how this would be done at the moment though...

Can you post a sample ember-cli-build.js with multiple EmberApps? I am interested how such would look like.

[pangratz]

I merged and released this in v0.3.0. Thanks @ef4! If you have time, can you post such a ember-cli-build.js with multiple EmberApps? Maybe we can add a test for this ...

[ef4]

Here's an example ember-cli-build.js with a second EmberApp:

var EmberApp = require('ember-cli/lib/broccoli/ember-app');
var stew = require('broccoli-stew');

module.exports = function(defaults) {
  var app = new EmberApp(defaults);
  var trees = [app.toTree()];
  if (EmberApp.env() === 'development') {
    trees.push(sampleApp().toTree());
  }
  return stew.find(trees);
};

function sampleApp(defaults) {
  return new EmberApp(defaults, {
    trees: {
      app: 'sample/app',
      public: 'sample/public',
      templates: null
    },
    tests: false,
    outputPaths: {
      app: {
        html: '/sample/index.html',
        css: {
          'app': '/sample/assets/sample.css'
        },
        js: '/sample/assets/sample.js'
      },
      vendor: {
        js: '/sample/assets/vendor.js',
        css: '/sample/assets/vendor.css'
      }
    }
  });
}

The use case here is that the main Ember app is actually a widget that gets embedded in an iframe inside other sites. The "sample" subdirectory contains a sample site that embeds the Ember app's iframe. With this setup, you can visit http://localhost:4200/sample/index.html to see the sample app.

Another use case I have seen is building a standalone Ember app for an admin area.

[pangratz]

Ah, ok. Interesting. Thanks.

[alexdiliberto]

This merge request is actually causing an issue with my App where I can't upgrade this addon to 0.3.0 or further. I have multiple EmberApp()'s within my ember-cli-build.js file, one for Desktop and one for Mobile.

pseudo code:

// Build out my-app.js and my-app.css
// Where require('./build/desktop') just `module.exports` a `new EmberApp()`
var desktopApp = require('./build/desktop')(defaults, appDefaults)

// Build out my-app-mobile.js and my-app-mobile.css
// Where require('./build/mobile') just `module.exports` a `new EmberApp()`
var mobileApp =  require('./build/mobile')(defaults, appDefaults)

if (process.env.MOBILE) {
  return mobileApp.toTree();
} else if (process.env.DESKTOP) {
  return desktopApp.toTree();
} else {
  return mergeTrees([mobileApp.toTree(), desktopApp.toTree()], {overwrite: true});
}

now because of this MR if condition my second tree will not get registered...Then I get just get a bunch of TestLoader errors for my integration tests

TestLoader Failures: my-app/tests/desktop/integration/components/my-component-test: could not be loaded (1, 0, 1)Rerun

Died
 on test #1 at Object.TestLoader.moduleLoadFailure (http://localhost:4200/assets/test-support.js:6693:11) at Object.TestLoader.require (http://localhost:4200/assets/test-loader.js:62:14) at Object.TestLoader.loadModules (http://localhost:4200/assets/test-loader.js:51:18)
 at Function.TestLoader.load (http://localhost:4200/assets/test-loader.js:82:24) at http://localhost:4200/assets/test-support.js:6702:16: Could not find module `htmlbars-inline-precompile` imported from `my-app/tests/desktop/integration/components/my-component-test`@
 0 ms

Source:     
Error: Could not find module `htmlbars-inline-precompile` imported from `my-app/tests/desktop/integration/components/my-component-test`
    at requireFrom (http://localhost:4200/assets/vendor.js:120:13)
    at reify (http://localhost:4200/assets/vendor.js:107:22)
    at mod.state (http://localhost:4200/assets/vendor.js:149:17)
    at tryFinally (http://localhost:4200/assets/vendor.js:31:14)
    at requireModule (http://localhost:4200/assets/vendor.js:148:5)
    at Object.TestLoader.require (http://localhost:4200/assets/test-loader.js:60:9)
    at Object.TestLoader.loadModules (http://localhost:4200/assets/test-loader.js:51:18)
    at Function.TestLoader.load (http://localhost:4200/assets/test-loader.js:82:24)
    at http://localhost:4200/assets/test-support.js:6702:16

@ef4 @pangratz

[pangratz]

@alexdiliberto a sample repository demonstrating the issue would be of great help...

[alexdiliberto]

@pangratz Sure, here's a very simple reproduction of the issue https://github.com/alexdiliberto/multi-emberapp-htmlbars-inline-precompile-bug.

One way to recreate is just to clone the repo and run scripts/desktop t or ember t. You'll see the issue when you run. For the quickest fix just remove one of the EmberApp's (so now there is only 1 EmberApp again)...so just comment out line 8 in ember-cli-build.js https://github.com/alexdiliberto/multi-emberapp-htmlbars-inline-precompile-bug/blob/master/ember-cli-build.js#L8. Now run scripts/desktop t and it works because there is only 1 EmberApp now

+@ef4

[pangratz]

Awesome @alexdiliberto, I will look into it.

[pangratz]

My insights into the plugin architecture of Ember-CLI are limited, but it seems that storing setting the _registeredWithBabel on the app instead of the plugin (aka this), solves the problem:

if (!app._registeredWithBabel) {
  app.options.babel.plugins.push(PrecompileInlineHTMLBarsPlugin);
  app._registeredWithBabel = true;
}

I am not sure if this is a valid and desired solution though. Maybe @rwjblue can chime in and tell if it is OK to set properties on the app (which I think it is not) and if there is a better workaround for the moment.

Eventually this will be working more cleanly once #9 is merged...

[alexdiliberto]

@pangratz Friendly ping. Been a few months...any update? This issue still blocks upgrade >= 0.3.0 for my App. You mentioned that #9 may help here

[pangratz]

Hey @alexdiliberto, unfortunately there hasn't been any progress on #9. I may have time over the holidays to give this a new shot.

Do you think it would be possible to apply the change I mentioned in my comment above locally within your app and see if this helps?

[alexdiliberto]

Hey @pangratz thanks for the quick update. So, I made that update within my prod app this morning and, like you said, it does actually fix the issue by registering on the app instead of the plugin. I hope that helps. Let me know if you have any other questions!

[pangratz]

OK, so it looks like this would solve this issue. Glad it works for you! Now we just need to find out if this solution is acceptable 😉