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

Make codemod target classes generically #3

Closed
pzuraq opened this issue Aug 2, 2018 · 3 comments
Closed

Make codemod target classes generically #3

pzuraq opened this issue Aug 2, 2018 · 3 comments

Comments

@pzuraq
Copy link
Collaborator

pzuraq commented Aug 2, 2018

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();
@ssutar
Copy link
Contributor

ssutar commented Aug 2, 2018

Thanks @pzuraq !

@scalvert and I were discussing about this. We were thinking about making this configurable to make it easier to roll out on a large codebase.

Specifically we were thinking to have an ability transform only specific object like services or controllers. This way users can run the codemods in iterations. For example, services in the first pass. controllers in another.

Additionally we were thinking about having options to exclude/include certain objects - like target objects which does not have any decorators.

Would love to hear your thoughts!

@pzuraq
Copy link
Collaborator Author

pzuraq commented Aug 2, 2018

Hmm, I wonder if we should have it be scoped by identifier, or by file path. Would it make sense to want to convert half of a file?

For instance, if we ran the codemod on just Controller then only half of this example would be converted:

const UtilityClass = EmberObject.extend({
  // ...
});

export default Controller.extend({
  // ...uses utility class 
});

Would that be more desirable, or would it be better to convert the entire file at once and allow users to scope by saying --include '/app/controllers/**/*'

For decorators vs. not-decorators, completely agree. This would essentially be the difference between stage-3+ and stage-2+, and could probably be a command line option (with the default being stage-3+).

@ssutar
Copy link
Contributor

ssutar commented Sep 7, 2018

#12

@pzuraq pzuraq closed this as completed Aug 16, 2019
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