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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

returning debug macro statements results in an error #168

Closed
toranb opened this issue Jul 26, 2017 · 4 comments
Closed

returning debug macro statements results in an error #168

toranb opened this issue Jul 26, 2017 · 4 comments

Comments

@toranb
Copy link

toranb commented Jul 26, 2017

I started the upgrade to ember-cli-babel v6.6.0 for one of my addons recently and found that when I destructure assert from @ember/debug the compiled output omits it for some odd reason. I'm writing to better understand if what I found is truly a regression of some kind or just me doing something I shouldn't be (and getting away with it prior to the v6.6.0 upgrade).

The import below is what doesn't work for dev or test builds any longer

import { assert } from '@ember/debug';

note: before using ember-cli-babel v6.6.0 I would pull assert from the global Ember

import Ember from 'ember';

const { assert } = Ember;

The difference is that prior to this import change and ember-cli-babel v6.6.0 my test build would run and assert would work as designed. Now I get an error saying assert is not defined when the test runs.

note: in dev tools when I capture this failure at test time and look at the imports / variable defs at the top of my module I don't see assert

screen shot 2017-07-26 at 6 53 07 am

Notice in the actual source for that file I have assert imported

screen shot 2017-07-26 at 6 56 13 am

I'm using ember-cli 2.13.0, ember 2.13.3 with node 6.10.2 and ember-cli-babel v6.6.0. For a full and complete repro checkout the branch I have up for the PR that bumps it to ember-cli-babel v6.6.0

@rwjblue
Copy link
Member

rwjblue commented Jul 26, 2017

The fact that the assert import is removed is not the issue, instead there is a problem with the prod/debug stripping plugin that we need to track down. Can you share the full transpiled output of embed-redux/connect module?

I currently suspect that computedReduxProperty using implicit returning arrow functions is the issue (Specifically, we don't handle the return assert(....) correctly). A work around for now that you could test would be to change computedReduxProperty's set into a block form arrow function (or plain function).

@toranb
Copy link
Author

toranb commented Jul 26, 2017

@rwjblue understood -thanks again for hearing me out (sorry to create such a verbose issue so early in the morning). Below you will find the full transpiled source as requested

define('ember-redux/connect', ['exports', 'redux'], function (exports, _redux) {
  'use strict';

  Object.defineProperty(exports, "__esModule", {
    value: true
  });

  var _typeof = typeof Symbol === "function" && typeof Symbol.iterator === "symbol" ? function (obj) {
    return typeof obj;
  } : function (obj) {
    return obj && typeof Symbol === "function" && obj.constructor === Symbol && obj !== Symbol.prototype ? "symbol" : typeof obj;
  };

  var run = Ember.run;
  var inject = Ember.inject.service;
  var Component = Ember.Component;
  var computed = Ember.computed;
  var getProperties = Ember.getProperties;
  var defineProperty = Ember.defineProperty;

  function changedKeys(props, newProps) {
    return Object.keys(props).filter(function (key) {
      return props[key] !== newProps[key];
    });
  }

  function computedReduxProperty(key, getProps) {
    return computed({
      get: function get() {
        return getProps()[key];
      },
      set: function set() {
        return assert('Cannot set redux property "' + key + '". Try dispatching a redux action instead.');
      }
    });
  }

  function getAttrs(context) {
    var keys = Object.keys(context.attrs || {});
    return getProperties(context, keys);
  }

  function wrapStateToComputed(stateToComputed) {
    return function () {
      var result = stateToComputed();
      if (typeof result === 'function') {
        stateToComputed = result;
        return stateToComputed();
      }
      return result;
    };
  }

  exports.default = function (stateToComputed) {
    var dispatchToActions = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : function () {
      return {};
    };

    return function (IncomingComponent) {
      var WrappedComponent = IncomingComponent || Component;
      return WrappedComponent.extend({
        redux: inject('redux'),

        init: function init() {
          var _this = this;

          var redux = this.get('redux');

          if (stateToComputed) {
            var getProps = wrapStateToComputed(function () {
              return stateToComputed.call(_this, redux.getState(), getAttrs(_this));
            });
            var props = getProps();

            Object.keys(props).forEach(function (key) {
              defineProperty(_this, key, computedReduxProperty(key, function () {
                return props;
              }));
            });

            this._handleChange = function () {
              var newProps = getProps();
              if (props === newProps) return;

              var notifyProperties = changedKeys(props, newProps);

              props = newProps;

              if (notifyProperties.length > 0) {
                run.join(function () {
                  notifyProperties.forEach(function (name) {
                    return _this.notifyPropertyChange(name);
                  });
                });
              }
            };

            this.unsubscribe = redux.subscribe(function () {
              _this._handleChange();
            });
          }

          if (typeof dispatchToActions === 'function') {
            this.actions = Object.assign({}, this.actions, dispatchToActions.call(this, redux.dispatch.bind(redux)));
          }

          if ((typeof dispatchToActions === 'undefined' ? 'undefined' : _typeof(dispatchToActions)) === 'object') {
            this.actions = Object.assign({}, this.actions, (0, _redux.bindActionCreators)(dispatchToActions, redux.dispatch.bind(redux)));
          }

          this._super.apply(this, arguments);
        },
        didUpdateAttrs: function didUpdateAttrs() {
          this._super.apply(this, arguments);
          if (this._handleChange) {
            this._handleChange();
          }
        },
        willDestroy: function willDestroy() {
          this._super.apply(this, arguments);
          if (this.unsubscribe) {
            this.unsubscribe();
            this.unsubscribe = null;
          }
        }
      });
    };
  };
});

@rwjblue
Copy link
Member

rwjblue commented Jul 26, 2017

OK, thanks for sharing. My suggested work around should work and avoid the errors you are seeing:

function computedReduxProperty(key, getProps) {
  return computed({
    get: () => getProps()[key],
    set: () => { assert(`Cannot set redux property "${key}". Try dispatching a redux action instead.`); }
  });
}

I have created ember-cli/babel-plugin-debug-macros#46 to track fixing the underlying babel plugin.

@rwjblue rwjblue changed the title How to import assert for dev/ test builds from @ember/debug Issue with returning debug macro statements (e.g. return assert(...)) Jul 26, 2017
@rwjblue rwjblue changed the title Issue with returning debug macro statements (e.g. return assert(...)) returning debug macro statements results in an error Jul 26, 2017
@toranb
Copy link
Author

toranb commented Jul 27, 2017

@rwjblue thanks for both the workaround and opening the real issue w/ babel-plugin-debug-macros

@toranb toranb closed this as completed Jul 27, 2017
siva-sundar pushed a commit to siva-sundar/ember-cli-babel that referenced this issue Feb 11, 2021
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

No branches or pull requests

2 participants