head-browser instance-initializer not found from ember-page-title, booting the app fails (v0.3, fastboot rc3)

[vlascik]

When using this addon with ember-page-title, it requires this addon with before: 'head-browser' in ember-page-title\app\instance-initializers\page-title-setup.js . However, this fails with:

Error
    at assert (P:\Project\tmp\broccoli_merge_trees-output_path-MnfEnoEm.tmp\assets\vendor\ember\ember.debug.js:13766:1)
    at P:\Project\tmp\broccoli_merge_trees-output_path-MnfEnoEm.tmp\assets\vendor\ember\ember.debug.js:12325:1
    at Vertices.each (P:\Project\tmp\broccoli_merge_trees-output_path-MnfEnoEm.tmp\assets\vendor\ember\ember.debug.js:10965:1)
    at Vertices.walk (P:\Project\tmp\broccoli_merge_trees-output_path-MnfEnoEm.tmp\assets\vendor\ember\ember.debug.js:10894:1)
    at DAG.each (P:\Project\tmp\broccoli_merge_trees-output_path-MnfEnoEm.tmp\assets\vendor\ember\ember.debug.js:10838:1)
    at DAG.topsort (P:\Project\tmp\broccoli_merge_trees-output_path-MnfEnoEm.tmp\assets\vendor\ember\ember.debug.js:10844:1)
    at Class._runInitializer (P:\Project\tmp\broccoli_merge_trees-output_path-MnfEnoEm.tmp\assets\vendor\ember\ember.debug.js:12341:1)
    at Class.runInstanceInitializers (P:\Project\tmp\broccoli_merge_trees-output_path-MnfEnoEm.tmp\assets\vendor\ember\ember.debug.js:12323:1)
    at Class._bootSync (P:\Project\tmp\broccoli_merge_trees-output_path-MnfEnoEm.tmp\assets\vendor\ember\ember.debug.js:11147:1)
    at P:\Project\tmp\broccoli_merge_trees-output_path-MnfEnoEm.tmp\assets\vendor\ember\ember.debug.js:12107:1
    at initializePromise (P:\Project\tmp\broccoli_merge_trees-output_path-MnfEnoEm.tmp\assets\vendor\ember\ember.debug.js:54170:1)
    at new Promise (P:\Project\tmp\broccoli_merge_trees-output_path-MnfEnoEm.tmp\assets\vendor\ember\ember.debug.js:54654:1)
    at Class.boot (P:\Project\tmp\broccoli_merge_trees-output_path-MnfEnoEm.tmp\assets\vendor\ember\ember.debug.js:12106:1)
    at buildAppInstance.then.appInstance (P:\Project\node_modules\fastboot\src\ember-app.js:240:25)
    at tryCatch (P:\Project\tmp\broccoli_merge_trees-output_path-MnfEnoEm.tmp\assets\vendor\ember\ember.debug.js:54120:1)
    at invokeCallback (P:\Project\tmp\broccoli_merge_trees-output_path-MnfEnoEm.tmp\assets\vendor\ember\ember.debug.js:54135:1)
    at publish (P:\Project\tmp\broccoli_merge_trees-output_path-MnfEnoEm.tmp\assets\vendor\ember\ember.debug.js:54103:1)
    at P:\Project\tmp\broccoli_merge_trees-output_path-MnfEnoEm.tmp\assets\vendor\ember\ember.debug.js:43528:1
    at invokeWithOnError (P:\Project\tmp\broccoli_merge_trees-output_path-MnfEnoEm.tmp\assets\vendor\ember\ember.debug.js:9122:1)
    at Queue.flush (P:\Project\tmp\broccoli_merge_trees-output_path-MnfEnoEm.tmp\assets\vendor\ember\ember.debug.js:9008:1)
    at DeferredActionQueues.flush (P:\Project\tmp\broccoli_merge_trees-output_path-MnfEnoEm.tmp\assets\vendor\ember\ember.debug.js:9171:1)
    at Backburner.end (P:\Project\tmp\broccoli_merge_trees-output_path-MnfEnoEm.tmp\assets\vendor\ember\ember.debug.js:9250:1)
    at Timeout.Backburner._boundAutorunEnd [as _onTimeout] (P:\Project\tmp\broccoli_merge_trees-output_path-MnfEnoEm.tmp\assets\vendor\ember\ember.debug.js:9219:1)
    at ontimeout (timers.js:386:14)
    at tryOnTimeout (timers.js:250:5)
    at Timer.listOnTimeout (timers.js:214:5)

While debugging runInstanceInitializers parameters I can see head-browser is undefined:
page-title-setup-browser { name: 'page-title-setup-browser',
before: 'head-browser',
initialize: [Function: initialize] }
head-browser undefined

Renaming ember-cli-head\app\instance-initializers\head.js to head-browser.js seems to fix the problem.

[kratiahuja]

@simonihmig have you seen this?

[simonihmig]

have you seen this?

No, have not used this addon.

@vlascik you are using ember-page-title with version 0.3.0 of ember-cli-head? Because ember-page-title itself still refers to 0.2.1 as its dependency.

Not sure what's the problem, because head-browser was never moved around (other than dropping the browser subfolder). And its internal name is still "head-browser": https://github.com/ronco/ember-cli-head/blob/master/app/instance-initializers/head.js#L24

[vlascik]

@simonihmig yes, I'm on 0.3.0, I'm using greenkeeper's PR for ember-page-title for that ember-cli/ember-page-title#72
name is still head-browser, that's ok, filename seems to be the problem. Maybe it's something that changed in later ember versions? I'm on cli 2.14.0-beta.2, ember 2.14.0-beta.1

[simonihmig]

@vlascik In that Greenkeeper PR all tests are passing though!? They would fail if the same initializer issue would come up in CI, wouldn't they?

Where do you see the mentioned error happening?

[vlascik]

@simonihmig This only shows up in fastboot, I'm not sure if or how they test for it.
I've created a minimal reproduction here: https://github.com/vlascik/head-reproduction . It's just a new app with fastboot and that PR added.
After ember serve http://localhost:4200/?fastboot=false works, http://localhost:4200/?fastboot=true leads to the error above.

[simonihmig]

I think I have found something...

In your reproduction repo in the app.js (head-reproduction.js) there are both instance-initializers as they should be (shortened code):

define('head-reproduction/instance-initializers/head', ...
 
  exports.default = {
    name: 'head-browser',
    initialize: ...
  };
});
define('head-reproduction/instance-initializers/page-title-setup', ...
  exports.default = {
    name: 'page-title-setup-browser',
    before: 'head-browser',
    initialize: ...
  };
});

So far this should be fine. However the FastBoot instance-initializer (https://github.com/ronco/ember-cli-head/blob/master/fastboot/instance-initializers/head.js), which lives in app-fastboot.js (head-reproduction-fastboot.js in this case), has the same module name:

define('head-reproduction/instance-initializers/head', ...
  exports.default = {
    name: 'head-fastboot',
    initialize: initialize
  };
});

So it is overriding the browser initializer`s module, and as it has a different initializer name, there is no "head-browser" initializer anymore. That is in FastBoot, in a browser the override does not happen obviously, so no problems there.

These conflicting changes were introduced with the FastBoot 1.0 migrating PRs #21 & #32 (cc @cibernox )

This is a special case, when you use the after/before properties. Otherwise it would do no harm, as only one of these initializers (browser/fastboot) has to run anyway. Will come up with a PR fixing this in a minute...

However this may be an issue other addons might run into as well, especially when migrating away from the fastboot-filter-initializers tool!

@kratiahuja your automatic initializer migration (https://github.com/ember-fastboot/ember-cli-fastboot/blob/master/lib/build-utilities/migrate-initializers.js) might possibly also cause similar problems, as it is changing the module name (removing the "fastboot" subfolder part), and thus possibly causing an existing initializer in /app to be hidden? Hopefully not a real world problem so far...

[kratiahuja]

@kratiahuja your automatic initializer migration (https://github.com/ember-fastboot/ember-cli-fastboot/blob/master/lib/build-utilities/migrate-initializers.js) might possibly also cause similar problems, as it is changing the module name (removing the "fastboot" subfolder part), and thus possibly causing an existing initializer in /app to be hidden?

The migration tool is just moving things around. It's not supposed to touch the files and change content. Ideally addons should be keeping the name property for both initializers (Node and browser). So yes with the new scheme addons can cause breakage but apps will fail fast and we will know. rc builds in general are a breaking change.

@simonihmig why not have the same name property for both browser and fastboot initializer in ember-cli-head? Instead of having head-browser and head-fastboot, just have head for both. This way it will work in both Node and browser (if page-title is supposed to function in Node too). Ofcourse page-title addon would need to be bumped. Therefore, if you have an addon that is dependent on ember-cli-head and needs to make sure it works in both Node and browser, would just need to know which initializer name to run before/after rather than deep diving in the environment too.

[simonihmig]

why not have the same name property for both browser and fastboot initializer in ember-cli-head

@kratiahuja I just wanted to prevent a breaking change because of this. Don't know if ember-page-title is the only addon that would be affected by this?

[kratiahuja]

At least that's what the code search on ember observer shows