ember-codemods / ember-native-class-codemod Goto Github PK

Summarizing various conversations we've had on this topic in this issue, pretty sure everyone is in alignment on the path forward.

In Ember today, computed properties can have modifiers which change the behavior of the computed:

foo: computed('bar', function() { /*...*/ })
  .readOnly() // makes the computed read only (throw on set)
  .volatile() // makes the computed volatile (recomputed each time)
  .property('baz') // adds additional dependent keys to the computed
  .meta({ type: 'Property' }); // adds metadata to the computed

These modifiers could be chained in any order on an instance of a computed property. However, they are not fully supported by ember-decorators yet, so we have to decide on an API to target.

Some additional constraints:

1. Decorators can only consist of A) a chain of identifiers with a final function call invocation at the very end or B) an arbitrary expression surrounded by parentheses:

   // valid
   @foo
   @foo.bar 
   @foo()
   @foo.bar()
   @(foo().bar)
   @(foo().bar())
   
   // invalid
   @foo().bar
   @foo().bar()

2. We don't want to lean too heavily on the ability to use expressions in parens, because it is more verbose, harder to read, and is counter to our long term goal of making Ember feel like JS

3. property and meta will require expressions no matter what because they take parameters, but they are very very rare use cases. property only makes sense for computed macros (otherwise the dependent key could be added to the computed() call) and meta is only known to be used by Ember Data.

4. We want the new system to be fairly intuitive to users of the old system

With that in mind, the proposal is the following:

// INPUT
foo: computed('bar', function() {}).readOnly()

// OUTPUT
// readOnly is very commonly used, so not requiring expression syntax here is good.
@computed.readOnly('bar') get foo() {}

// INPUT
foo: alias('bar').readOnly()

// OUTPUT
@alias.readOnly('bar') foo;

// INPUT
foo: computed('bar', function() {}).volatile()

// OUTPUT
// volatile is fairly commonly used, so not requiring expression syntax here is good.
// Dependent keys are meaningless to volatile computeds, so they can be removed
@computed.volatile() get foo() {} 

// INPUT
foo: computed('bar', function() {}).readOnly().volatile()

// OUTPUT
// volatile getters behave the same as native getters, and because the property is readOnly 
// we can know without a doubt that it can be replaced safely with a native getter. This 
// prevents us from having to support chaining in non-expression computed usage
get foo() {} 

// INPUT
foo: alias('bar').property('baz')

// OUTPUT
// requires expression syntax due to params
@(alias('bar').property('baz')) foo;

// INPUT
foo: computed('bar', function() {}).meta({ type: 'Property' })

// OUTPUT
// requires expression syntax due to params
@(computed('bar').meta({ type: 'Property' })) get foo() {}

volatile generally doesn't make sense with macros, so we shouldn't have to account for that situation. property and meta should be chainable in any order, and should be able to chain off of readOnly and volatile as well.

I ran Dyfactor to gather telemetry data and found that there was a mismatch between what the codemod expects and what Dyfactor outputs for file identifiers. The codemod seems to look for relative file paths (e.g. app/adapters/application) but it ended up with full filepaths:

     "/Users/cgarrett/Code/emberobserver/client/app/adapters/application.js": {
        "computedProperties": ["headers"],
        "observedProperties": [],
        "observerProperties": {},
        "offProperties": {},
        "overriddenActions": [],
        "overriddenProperties": [
          "coalesceFindRequests",
          "shouldBackgroundReloadRecord",
          "shouldBackgroundReloadAll"
        ],
        "ownProperties": [
          "namespace",
          "coalesceFindRequests",
          "headers",
          "shouldBackgroundReloadRecord",
          "shouldBackgroundReloadAll"
        ],
        "type": "EmberObject",
        "unobservedProperties": {}
      },

Command:

yarn dyfactor run template ember-object app --level extract

myThing: localComputedMacro('foo');

becomes

@localComputedMacro
myThing;

foo is missing

Currently, the import transform for services is incorrect.

import { inject as service } from '@ember/service';

is replaced with:

import { service } from "@ember-decorators/service";

I believe it should be:

import { inject as service } from "@ember-decorators/service";

Missing following decorator imports

{
  "@ember-decorators/object": ["action", "readOnly", "volatile"],
  "@ember-decorators/component": [
    "layout",
    "className",
    "classNames",
    "tagName",
    "attribute"
  ]
};

In the current implementation, the codemods exit if it encounters a property having value CallExpression or ObjectExpression which can not be handled via decorators.

While this works in most cases, we need to make a exception to this validation rule to handle queryParams

const a = Foo.extend({
  queryParams: { ... }
})

need to be transformed to

class A  extends Foo {
  queryParams = { ... }
}

Input:

import Service from "@ember/service";
import Controller from "@ember/controller";

const ser = Service.extend({});
const ctrl = Controller.extend({});

Expected:

import Service from "@ember/service";
import Controller from "@ember/controller";

class Ser extends Service {}
class Ctrl extends Controller {}

Actual result:

import Service from "@ember-decorators/service";
import Controller from "@ember-decorators/controller";

class Ser extends Service {}
class Ctrl extends Controller {}

In recent execution of codemods on different groups of files, a weird behavior is observed - It sometimes update the template file as well thereby creating a syntax error

The reason it happens is we run

root.toSource();

even if the file we're transforming has validation failed.

Adding a change to transform conditionally based on result of validation

I enabled Travis, but it doesn't seem like tests are running on CI yet. We should get this up and running asap to make sure our code is correct.

I ran this on a fairly sizable project (it transformed 309 files). Two of those files were templates. Here are the changes that were made:

I ran the following command:

ember-es6-class-codemod ember-object --decorators=true app/**

With my app running on localhost:4200, and running the codemod like so:

npx ember-native-class-codemod http://localhost:4200/ app

I see the following errors:

npx ember-native-class-codemod http://localhost:4200/ app
Downloading Chromium r669486 - 106.8 Mb [====================] 100% 0.0s
error evaluating `ember-composable-helpers/helpers/append`: Cannot read property '1' of undefined
error evaluating `ember-composable-helpers/helpers/drop`: Cannot read property '1' of undefined
error evaluating `ember-composable-helpers/helpers/flatten`: Cannot read property '1' of undefined
error evaluating `ember-composable-helpers/helpers/intersect`: Cannot read property '1' of undefined
error evaluating `ember-composable-helpers/helpers/join`: Cannot read property '1' of undefined
error evaluating `ember-composable-helpers/helpers/reverse`: Cannot read property '1' of undefined
error evaluating `ember-composable-helpers/helpers/shuffle`: Cannot read property '1' of undefined
error evaluating `ember-composable-helpers/helpers/slice`: Cannot read property '1' of undefined
error evaluating `ember-composable-helpers/helpers/take`: Cannot read property '1' of undefined
error evaluating `ember-composable-helpers/helpers/union`: Cannot read property '1' of undefined
error evaluating `ember-moment/helpers/-base`: Cannot read property '1' of undefined
error evaluating `ember-moment/helpers/moment-format`: Cannot read property '1' of undefined
error evaluating `torii/load-initializers`: Could not find module `torii/initializers/initialize-torii` imported from `torii/load-initializers`

and then the codemod outputs an error:

/Users/katie/.npm/_npx/20280/lib/node_modules/ember-native-class-codemod/node_modules/jsonfile/index.js:74
      throw err;
      ^

Error: tmp/broccoli_persistent_filtersimple_replace-input_base_path-IedTr0tn.tmp/package.json: ENOENT: no such file or directory, open 'tmp/broccoli_persistent_filtersimple_replace-input_base_path-IedTr0tn.tmp/package.json'
    at Object.openSync (fs.js:447:3)
    at Object.readFileSync (fs.js:349:35)
    at Object.readFileSync (/Users/katie/.npm/_npx/20280/lib/node_modules/ember-native-class-codemod/node_modules/jsonfile/index.js:61:22)
    at Object.readJsonSync (/Users/katie/.npm/_npx/20280/lib/node_modules/ember-native-class-codemod/transforms/helpers/util/get-telemetry-for.js:16:16)
    at Module._compile (internal/modules/cjs/loader.js:774:30)
    at Module._compile (/Users/katie/.npm/_npx/20280/lib/node_modules/ember-native-class-codemod/node_modules/pirates/lib/index.js:99:24)
    at Module._extensions..js (internal/modules/cjs/loader.js:785:10)
    at Object.newLoader [as .js] (/Users/katie/.npm/_npx/20280/lib/node_modules/ember-native-class-codemod/node_modules/pirates/lib/index.js:104:7)
    at Module.load (internal/modules/cjs/loader.js:641:32)
    at Function.Module._load (internal/modules/cjs/loader.js:556:12) {
  errno: -2,
  syscall: 'open',
  code: 'ENOENT',
  path: 'tmp/broccoli_persistent_filtersimple_replace-input_base_path-IedTr0tn.tmp/package.json'
}

and then ends with:

All done.
Results:
0 errors
0 unmodified
0 skipped
0 ok

But nothing has been modified.

Given:

// I'm using pods structure
// app/components/lorem-ipsum/component.js
import Component from '@ember/component';

export default Component.extend({
  classNameBindings: ['obj.key:value'],
});

Result:

import Component from '@ember/component';

export default class Component extends Component {}

Expected:

import Component from '@ember/component';
import { attribute, className } from '@ember-decorators/component';

export default class LoremIpsumComponent extends Component {
  @className
  @computed('obj.key')
  get classNameComputed() {
    if (this.obj.key) return 'value';
  }
}

The map and filter computed props expects parameters to be passed. In this case we don't need to convert it to native getters

import { map } from '@ember/object/computed';
import EmberObject from '@ember/object';

let Hamster = EmberObject.extend({
  excitingChores: map('chores', function(chore, index) {
    return chore.toUpperCase() + '!';
  })
});

should be transformed to

import { map } from '@ember-decorators/object/computed';
import EmberObject from '@ember/object';

class Hamster extends EmberObject {
  @map('chores', function(chore, index) {
    return chore.toUpperCase() + '!';
  })
  excitingChores;
};

Consistently across my codebase, every time a service injection was used, it was replaced by the codemod with two @service decorators.

Example:

Before:

import { inject as service } from '@ember/service';
import Component from '@ember/component';
import { readOnly } from '@ember/object/computed';

export default Component.extend({
  classNames: 'UpgradeSuccess',
  userService: service('user'),
  user: readOnly('userService.model')
});

After:

import classic from 'ember-classic-decorator';
import { classNames } from '@ember-decorators/component';
import { inject as service } from '@ember/service';
import { readOnly } from '@ember/object/computed';
import Component from '@ember/component';

@classic
@classNames('UpgradeSuccess')
export default class ComponentComponent extends Component {
  @service
  @service
  userService;

  @readOnly('userService.model')
  user;
}

I guess ember-es6-class-codemod is trying to name classes corresponding to their file names.

I'm using pods and I'm having following structure

• app/pods/authenticated
  • controller.js
  • route.js
  • template.hbs

File route.js was transformed into this

Result:

import Route from '@ember/routing/route';

export default class Route extends Route {
  // ...
}

So I received error Route is already defined

Expected:

import Route from '@ember/routing/route';

export default class AuthenticatedRoute extends Route {
  // ...
}

Non literal last parameter on a computed property stripped off. This is done as the last parameter is typically function or object which should be removed. However it does break the following case:

isEqualToLimit: equal("limit", NUMERIC_CONSTANT.LIMIT).readOnly(),

is transformed to

isEqualToLimit: equal("limit").readOnly(),

Here, the NUMERIC_CONSTANT.LIMIT is removed which is incorrect.

An action like below:

actions: { 
  onClick() { 
    if (this.onClick) { 
      this.onClick(...arguments); 
    } 
  },
}

With current codemods it gets transformed to

@action
onClick() {
  if (this.onClick) {
    this.onClick(...arguments);
  }
}

which goes into infinite loop

Here is the changelog: https://github.com/ember-decorators/ember-decorators/releases/tag/v5.0.0

In order to ensure that we don't migrate things incorrectly, we should add concatenatedProperties and mergedProperties to the list of runtime data that we collect.

If we detect "non-standard" entries in these lists, we should avoid migrating that class/file/whatever.

Need to add/replace the import statements with importing decorators from https://github.com/ember-decorators/ember-decorators

The following file did not transform, and all of the properties seemed to trigger errors:

import { computed } from '@ember/object';
import { gt } from '@ember/object/computed';
import Model from 'ember-data/model';
import attr from 'ember-data/attr';
import { hasMany, belongsTo } from 'ember-data/relationships';
import moment from 'moment';

export default Model.extend({
  isAddon: true,
  name: attr('string'),
  description: attr('string'),
  repositoryUrl: attr('string'),
  latestVersionDate: attr('date'),
  publishedDate: attr('date'),
  license: attr('string'),
  isDeprecated: attr('boolean'),
  note: attr('string'),
  isOfficial: attr('boolean'),
  isCliDependency: attr('boolean'),
  isHidden: attr('boolean'),
  hasInvalidGithubRepo: attr('boolean'),
  score: attr('number'),
  ranking: attr('number'),
  githubUsers: hasMany('github-user'),
  lastMonthDownloads: attr('number'),
  isWip: attr('boolean'),
  isTopDownloaded: attr('boolean'),
  isTopStarred: attr('boolean'),
  demoUrl: attr('string'),
  updatedAt: attr('date'),
  githubStats: belongsTo('github-stats', { async: true }),
  latestAddonVersion: belongsTo('version', { async: true }),
  latestReview: belongsTo('review', { async: true }),
  categories: hasMany('category', { async: true }),
  keywords: hasMany('keyword', { async: true }),
  versions: hasMany('version', { async: true }),
  maintainers: hasMany('maintainer', { async: true }),
  readme: belongsTo('readme', { async: true }),
  hasMoreThan1Contributor: gt('githubUsers.length', 1),
  npmUrl: computed('name', function() {
    return `https://www.npmjs.com/package/${this.get('name')}`;
  }),
  isNewAddon: computed('publishedDate', function() {
    return moment(this.get('publishedDate')).isAfter(moment().subtract(2, 'weeks'));
  })
});

We should add a guide which walks users through the usage of this codemod. It should walk users through:

1. Installing the codemod (and any additional dependencies)
2. Booting up Dyfactor to gather telemetry data
3. Running the codemod, including the options available
4. Removing the codemod from their package dependencies. The codemod should never be checked into the user's codebase.

The guide should assume that users want to convert their entire app/addon, and present options for scoping and such later on.

Objective:

• Every JS/TS file that was not migrated should be listed in the log
• The log consumer should be considered the developer and any content should be understandable and actionable
• When migration for a file is "bailed out" we should log the line and column number of the "offending" code that caused us to bail

Details

The log is helpful, but the messages can be a little bit unclear. Going to list out examples here as I find them:

• When the object cannot be transformed because it has an unknown computed assigned:

Transform not supported - need option '--decorators=true' or the property type CallExpression can not be transformed

The --decorators option was being used so that shouldn't be part of the message, and the message should indicate that there was property that we couldn't transform, and ideally the name of the property.

Example input:

import Component from '@ember/component';

export default Component.extend({
  tagName: '',
  alignBaseline: true,
}).reopenClass({
  positionalParams: ['iconName']
});

Currently we ignore a class definition if it supports mixins. We should be able to support this by using an intermediate extend:

// before
const Foo = EmberObject.extend(MyMixin, {
  // class definition
});

// after
class Foo extends EmberObject.extend(MyMixin) {
  // class definition
}

Having:

// - app/
//   - services/
//     - actions/
//       - user.js
//     - dude.js

import Service, { inject as service } from '@ember/service';
export default Service.extend({
  actions: service('actions/user'),
});

Result:

import { action } from "@ember-decorators/object";
import Service from '@ember/service';
import { service } from "@ember-decorators/service";
export default class Dude extends Service {
  @service('actions/user')
  actions;
}

Expected:

import Service from '@ember/service';
import { service } from "@ember-decorators/service";
export default class Dude extends Service {
  @service('actions/user')
  actions;
}

Currently the codemods import on from @ember-decorators/object/evented which doesn't even exist 😄

As per the decorators documentation it should be imported from @ember-decorators/object

Opening this issue to start tracking the various inputs and outputs of this transform. I think the list I have here is pretty comprehensive so far, let me know if you know any other edge cases or features.

Base

// Input
const Foo = EmberObject.extend({
  prop: 'defaultValue',
  
  method() {
    // do things
  },

  otherMethod: function() {

  },

  get accessor() {
    return this._value;
  },

  set accessor(value) {
    this._value = value;
  },
});

// Output
class Foo extends EmberObject {
  prop = 'defaultValue';

  method() {
    // do things
  }

  otherMethod() {

  }

  get accessor() {
    return this._value;
  }

  set accessor(value) {
    this._value = value;
  }
}


// Input
const Foo = EmberObject.extend(MyMixin, {
  // ...
});

// Output
class Foo extends EmberObject.extend(MyMixin) {
  // ...
}


// Throws error
const Foo = EmberObject.extend({
  boundMethod: () => {} // This is likely a bug, it would bind to module context
});

Stage 1

// Throws error
const Foo = EmberObject.extend({
  computed: computed({
    get() {},
    set() {},
  })
});

// Throws error
const Foo = EmberObject.extend({
  statefulObject: {},
  statefulArray: [],
});

// Throws error
const Foo = Component.extend({
  tagName: 'div',
});

// Throws error
const Foo = Component.extend({
  classNames: [],
});

// Throws error
const Foo = Component.extend({
  classNameBindings: ['foo'],
  foo: 'val',
});

// Throws error
const Foo = Component.extend({
  attributeBindings: ['foo'],
  foo: 'val',
});

// Throws error
const Foo = EmberObject.extend({
  actions: {
    bar() {}
  },
});

// Throws error
const Foo = EmberObject.extend({
  observer: observer(...)
});

// Throws error
const Foo = EmberObject.extend({
  event: on(...)
});

// Throws error
const Foo = EmberObject.extend({
  event: service(...)
});

// Throws error
const Foo = Controller.extend({
  event: controller(...)
});

// Shows dialog
// a. primitive value
// b. stateful object
// c. computed
// d. observer
// e. eventhandler
// Throws error if not a primitive value
const Foo = EmberObject.extend({
  macroValue: macro()
});

Stage 2

// Input
const Foo = EmberObject.extend({
  computed: computed({
    get() {},
    set() {},
  })
});

// Output
class Foo extends EmberObject {
  @computed
  get computed() {}

  set computed() {}
}


// Throws error
const Foo = EmberObject.extend({
  statefulObject: {},
  statefulArray: [],
});


// Input 
const Foo = Component.extend({
  tagName: 'div',
});

// Output
@tagName('div')
class Foo extends Component {}


// Input
const Foo = Component.extend({
  classNames: ['foo'],
});

// Output
@classNames('foo')
class Foo extends Component {
}


// Input
const Foo = Component.extend({
  classNameBindings: ['foo'],
  foo: 'val',
});

// Output
class Foo extends Component {
  @className foo = 'val';
}


// Input
const Foo = Component.extend({
  attributeBindings: ['foo'],
  foo: 'val',
});

// Output
class Foo extends Component {
  @attribute foo = 'val';
}


// Input
const Foo = EmberObject.extend({
  actions: {
    bar() {}
  },
});

// Output
class Foo extends EmberObject {
  @action
  bar() {}
}


// Input
const Foo = EmberObject.extend({
  observer: observer(...)
});

// Output (throws warning about @off)
class Foo extends EmberObject {
  @observes(...)
  observer() {}
}

// Input
const Foo = EmberObject.extend({
  event: on(...)
});

// Output (throws warning about @off)
class Foo extends EmberObject {
  @on(...)
  event() {}
}

// Input
const Foo = EmberObject.extend({
  myService: service(...)
});

// Output
class Foo extends EmberObject {
  @service(...) myService;
}


// Input
const Foo = Controller.extend({
  myController: controller(...)
});

// Output
class Foo extends Controller {
  @controller(...) myController;
}


// Shows dialog
// a. primitive value
// b. stateful object
// c. computed
// d. observer
// e. eventhandler
// Follows behavior based on user input
const Foo = EmberObject.extend({
  macroValue: macro()
});

Ember provides lifecycle hooks and event handlers for ember objects. However it was valid that action to have same name as any lifecycle hook.

Hooks and events: https://emberjs.com/api/ember/release/classes/Component

In the below example, its valid to have the click action as it is wrapped inside actions hash, and won't clobber with the event listener click

let Foo = Component.extend({
  click(event) {
  },
  actions: {
    click(action) {
    }
  }
})

When this code will be transformed to the ES6 class it will have 2 class properties with same name which will lead to syntax error.

We need to put validation to not to transform the class if it encounters any of the event handlers and methods defined as actions. The appropriate error will be logged in the codemods.log so that error is fixed before running the codemods again on that file.

Currently the codemod targets only calls to EmberObject.extend specifically, which excludes any subclasses of EmberObject including Ember concepts (Services, Routes, Components, etc).

We should be able to target any calls to .extend, but to be safe we can also scope it CapitalCased elements:

// transformed
FooClass.extend({});

// ignored
fooClass.extend({});
$.extend();

In some files the name of class is conflicting with the variables in the scope.

For example, the below code if the file name is foo.js,

import Foo from 'ember-foo-service/services/foo'

export default Foo.extend({ ... })

is transformed as

import Foo from 'ember-foo-service/services/foo'

class Foo extends Foo({ ... })

This is incorrect and would result in error

How do we handle this use case?

1. Figure out the variables in the current scope.
2. If there is a conflict with the generated class name

• Either use the containing folder name as a prefix to the class name. For example if the folder path is bar-biz/foo.js use BarBizFoo as class name
• If the conflict is not resolved in the above step, we should error out (or put a warning)

Dyfactor allows us to modify and instrument application code to gather dynamic information about an application that would be difficult or impossible to gather using static analysis alone. We can use Dyfactor to load every class in an application and gather information about them for the codemod, including:

• All of the properties and methods of the finalized class
• Which properties are computed properties
• Which methods are event listeners/observers

Using this information we can solve a few of the problems we have so far:

• Converting _super() calls to super.method() calls. With native classes, we can only call super if the super method actually exists, whereas the old _super() method would fail silently. If we can lookup the finalized parent we can check if the method exists before we transform it.
• Adding @unobserves and @off calls to overridden event listeners and observers. We must add these decorators on child classes that override methods which are decorated, and being able to lookup the parent to see if an existing observer/listener is there would be the easiest way to do that.

There will probably be other ways we can use this data as well. It should also be noted that a multipass codemod would probably not be as effective as Dyfactor, due to the usage of mixins in Ember. Without actually running the code, it's very difficult to determine what a finalized class with many mixins in its hierarchy would look like.

Before codemod:

export default Component.extend({
  fetchAlerts: task(function*() {
    let alerts = yield this.store.query('alert', {
      filter: { id: this.get('alert.id') }
    });
    return alerts.sortBy('createdAt').reverse();
  }).drop(),
});

after codemod:

@classic
export default class AlertHistoryComponent extends Component {
  @task
  fetchAlerts;
}

When transforming the Router with telemetry, it appends the type to the class name, which is incorrect.

We may want to come back to observers/event listeners. The trickiness is in subclasses, we must apply the opposite if the field is overriden:

const Foo = EmberObject.extend({
  fooObserver: observer('foo', function() {})
});

const Bar = Foo.extend({
  fooObserver: null, // unsets the observer
});

class Foo extends EmberObject {
  @observes('foo') fooObserver() {}
}

class Bar extends Foo {
  @unobserves('foo') fooObserver = null;
}

This will mean we have to add a second pass to the codemod, since we can't know what the subclass/superclass is until we've completed the first pass.

It looks likes it's href'd to a README.md that doesn't exist

Some functions/methods are missing jsdoc comments. Need to add documentation

In the file decorators.output.js the following imports should be combined

import { action, computed } from "@ember/object";
...
import { get, set } from "@ember/object";

We used to rename the imported variable in case layout decorator has naming conflict.

There were 2 issues with the previous approach:

1. All occurrences of the variable need to be renamed in the current file, its hard to determine the local/global scoped var

2. It was not able to handle the cases where the layout is passed down to the child components. Not sure if this is standard pattern but I've seen few examples of it.

Its better to rename the decorator itself so that we don't have to update the local vars which might introduce unexpected behavior

So instead of transforming to

import { layout } from "@ember-decorators/component";
import templateLayout from "components/templates/foo";

@layout(templateLayout)
class Foo extends EmberObject {}

it should be:

import { layout as templateLayout } from "@ember-decorators/component";
import layout from "components/templates/foo";

@templateLayout(layout)
class Foo extends EmberObject {}

The @classNames accepts strings as arguments. Currently codemods pass in an array of strings causing errors

Current behavior

@classNames(['class1', 'class2', ...])

should be changed to

@classNames('class1', 'class2', ...)

The transform errors out if we have actions like

import someActionUtil from 'some/action/util';

const Foo = Component.extend({
  actions: {
     someActionUtil
  }
});

It should be handled correctly

import someActionUtil from 'some/action/util';

const Foo = Component.extend({
   @action
   someActionUtil;
});

The super.method throws an error if the method is not present in the parent class. Unlike this._super which fails silently.

We can wrap the calls like:

if (super.method) {
  super.method();
}

#118 significantly changed how the codemod is intended to be used, we need to update the README to reflect it:

• No need to manually gather telemetry
• No need to install / setup dyfactor
• No need to pass ember-object as an argument during invocation (it is defaulted)
• Have to pass URL to running app (so that we can gather telemetry)

Class fields are currently defined as stage 3 proposal in the ECMA TC39 process. So they need to be provided as configurable option in the transforms.

It would have boolean value, default would be false When set to true it will transform the object properties to class fields. If false it will skip the object.

The @wrapComputed does not work correctly. For example in the following scenario the customMacro should be wrapped in @wrapComputed but it wraps all other calls as well.

  numPlusOne: computed("numProp", function() {
    return this.get("numProp") + 1;
  }),

  numPlusPlus: alias("numPlusOne"),

  computedMacro: customMacro(),

is transformed to

  @computed(computed("numProp", function() {
      return this.get("numProp") + 1;
  }))
  @wrapComputed(computed("numProp", function() {
      return this.get("numProp") + 1;
  }))
  numPlusOne;
    
  @alias(alias("numPlusOne"))
  @wrapComputed(alias("numPlusOne"))
  numPlusPlus;

  @wrapComputed(customMacro())
  computedMacro;

Expected output:

  @computed("numProp")
  get numPlusOne() {
    return this.get("numProp") + 1;
  }

  @alias("numPlusOne")
  numPlusPlus;

  @wrapComputed(customMacro())
  computedMacro;

A code like this causes issues since the clobbering of layout decorator and property

import layout from "components/templates/foo";
const Foo = EmberObject.extend({
  layout
});

This will result in

import { layout } from "@ember-decorators/component";
import layout from "components/templates/foo";

@layout(layout)
class Foo extends EmberObject {}

Which can be fixed in 2 ways

1. Change the default imported template specifier

import { layout } from "@ember-decorators/component";
import templateLayout from "components/templates/foo";

@layout(templateLayout)
class Foo extends EmberObject {}

2. Change the decorator import

import { layout as decoLayout } from "@ember-decorators/component";
import layout from "components/templates/foo";

@decoLayout(layout)
class Foo extends EmberObject {}

I like Option 1 as it does not change the decorator name

We need to add support for having multiple decorators on single property.

class Foo extends EmberObject {
@on("click")
@observes("xyz")
someProp = 'some value'
}