ember-fastboot / ember-cli-head

Ember Cli Addon for Adding Content to HTML Head
MIT License
98 stars 34 forks source link

Fix duplicate initializers appearing in dist js file #28

Closed slannigan closed 7 years ago

slannigan commented 7 years ago

(Note: This error could only be seen when using a project using https://github.com/ronco/ember-cli-meta-tags. If just running ember-cli-meta-tags, this error doesn't appear.)

The error:

When running ember s on our project (which we'll call project-name), we would get this console error: "Assertion Failed: The instance initializer 'head-browser' has already been registered". (https://puu.sh/w7HJm/fd018e7dc1.png)

On investigation, in dist/assets/project-name.js, there were two initializers when there should only have been one, which is what was causing this error to appear.

The first one:

define('project-name/instance-initializers/head', ['exports', 'ember', 'bidvine-web/config/environment'], function (exports, _ember, _environment) {
  'use strict';

  Object.defineProperty(exports, "__esModule", {
    value: true
  });
  exports.initialize = undefined;
  function _initialize(owner) {
    if (_environment.default['ember-cli-head'] && _environment.default['ember-cli-head']['suppressBrowserRender']) {
      return true;
    }

    // clear fast booted head (if any)
    _ember.default.$('meta[name="ember-cli-head-start"]').nextUntil('meta[name="ember-cli-head-end"] ~').addBack().remove();

    var component = owner.lookup('component:head-layout');
    component.appendTo(document.head);
  }

  exports.initialize = _initialize;
  exports.default = {
    name: 'head-browser',
    initialize: function initialize() {
      if (typeof FastBoot === 'undefined') {
        _initialize.apply(undefined, arguments);
      }
    }
  };
});

The second one:

define('project-name/instance-initializers/browser/head', ['exports', 'ember', 'bidvine-web/config/environment'], function (exports, _ember, _environment) {
  'use strict';

  Object.defineProperty(exports, "__esModule", {
    value: true
  });
  exports.initialize = initialize;
  function initialize(owner) {
    if (_environment.default['ember-cli-head'] && _environment.default['ember-cli-head']['suppressBrowserRender']) {
      return true;
    }

    // clear fast booted head (if any)
    _ember.default.$('meta[name="ember-cli-head-start"]').nextUntil('meta[name="ember-cli-head-end"] ~').addBack().remove();

    var component = owner.lookup('component:head-layout');
    component.appendTo(document.head);
  }

  exports.default = {
    name: 'head-browser',
    initialize: initialize
  };
});

My guess of what the problem is

Between 0.2.0 and 0.2.1, the non-fastboot initializers were moved from {{appFolderName}}/instance-initializers/browser/head.js to {{appFolderName]}/instance-initializers/head.js, while the fastboot initializers remained in {{appFolderName}}/instance-initializers/fastboot/head.js.

In ember-cli-head/app/index.js, broccoli-merge-trees is used to select the path to be used for the initializer. I think the different folder depth level of the fastboot vs non-fastboot initializers is causing broccoli-merge-trees to not merge the paths correctly, causing two initializers to be in the final distributed file.

The fix

Moved the non-fastboot initializers back to their original folders, in {{appFolderName}}/instance-initializers/browser/head.js. Regardless of what the precise cause is, I tested this solution and it does indeed fix the problem.

rwjblue commented 7 years ago

/cc @simonihmig @kratiahuja

kratiahuja commented 7 years ago

There doesn't exists app/instance-initializer/browser/head.js any longer so how is that getting built for you?

Can you put in a minimal reproduction with only ember-cli-head in here? Also what version of ember are you using?

simonihmig commented 7 years ago

The removal of the initializers within a browser folder is due to FastBoot 1.0 not supporting this anymore. And you can see this in the failing tests of your PR, the e2e tests for FastBoot 1.0 are failing.

What is puzzling me is that the code sample you posted for the second initializer does not even exist in this repo in the current master branch (and so in release 0.2.1). Search for "head-browser" (https://github.com/ronco/ember-cli-head/search?utf8=✓&q=head-browser&type=), and you see that there are two initializers, however both have a FastBoot guard around their initialize function, while you second code sample does not have it. Looks like an older implementation (prior to #21). I have no idea where that comes from!?

I don't think there can be two addons of the same name (different versions) active, right? Did your app maybe try to override instance-initializers/browser/head.js? Other than that, my last resort would be rm -rf node_modules tmp and reinstall/rerun... 🤔

kratiahuja commented 7 years ago

@slannigan can you create a minimal reproduction with a dummy app? This will help us nail down the issue to come up with the correct solution.

danconnell commented 7 years ago

The issue is we were on v3.1.6 of https://github.com/tim-evans/ember-page-title which was tied to 0.2.0 of ember-cli-head. With https://github.com/tim-evans/ember-page-title/pull/70 (v3.2.0), we no longer have this issue.

danconnell commented 7 years ago

To be clear, I assume it was the conflict of having both ember-page-title and ember-cli-meta-tags in our repo with them pointing at 2 different versions of ember-cli-head which was causing the issue.