ember-cli / ember-compatibility-helpers

Helpers that allow you to write backwards compat Ember addons
MIT License
24 stars 20 forks source link

Avoid duplicate plugin assertion. #36

Closed rwjblue closed 4 years ago

rwjblue commented 4 years ago

This is a fairly rare issue, but when it does happen it is quite difficult to debug.

The specific setup is:

module.exports = {
  name: require('./package').name,

  options: {
    m3: true,
    babel: {
      loose: true,
    },
  },
};

In this scenario, addon A will be instantiated twice. Once for addon B's dummy app and again for addon B's direct dependency. Since addon A had a mutable object on its prototype (yes, you know whats coming now) ember-compatibility-helpers adds a plugins array to it and pushes into it. Even though it sets a local flag saying that _registeredWithBabel, that flag is per instance of ember-compatibility-helpers and in our scenario we actually have two instances (therefore making our check moot).

The fixes here, prevent adding our comparison and debug plugins twice by checking if they are already present before pushing (it also adds a name property, which helps as well).

rwjblue commented 4 years ago

FYI - I wanted to use ember-cli-babel-plugin-helpers to do this, but it does not work when you want to search for a specific name within the plugins array. I created https://github.com/dfreeman/ember-cli-babel-plugin-helpers/issues/5 to track that.

rwjblue commented 4 years ago

CI is getting trolled by:

makepanic commented 4 years ago

This fixes the issue for us when upgrading ember-data to 3.14 in a monorepo.

pzuraq commented 4 years ago

Let's bump to Node 8 and release a new major

rwjblue commented 4 years ago

👍

omairvaiyani commented 4 years ago

Fixed our issue, the culprit addon was @ember/ordered-set

gabrielcsapo commented 4 years ago

Is this something we can get merged? We are getting duplicate addons in our product.

gabrielcsapo commented 4 years ago

Looks like the async is coming from broccoli-rollup which is from ember-data

  - stack: /home/travis/build/pzuraq/ember-compatibility-helpers/node_modules/broccoli-rollup/dist/index.js:31
603    async build() {
=> Found "broccoli-rollup@2.1.1"
info Reasons this module exists
   - "ember-data" depends on it
   - Hoisted from "ember-data#broccoli-rollup"
gabrielcsapo commented 4 years ago

This might need to be rebased to get the CI fixes and rerun to get passing.

rwjblue commented 4 years ago

Rebased and pushed...

rwjblue commented 4 years ago

OK, finally green (thanks all).

@pzuraq - Mind merge + releasing (I'm on mobile)?