ember-codemods / ember-codemods-telemetry-helpers

MIT License
6 stars 18 forks source link

Support for in-repo-addon files in `app/` (not `addon/`)? #19

Open raycohen opened 4 years ago

raycohen commented 4 years ago

I have a large ember app with several in-repo-addons that use the app/components/ folder for component implementation files, rather than having the implemenetation in addon/components/ and the re-export in app/components

When I run the ember-native-class-codemod against our in-repo app, I get errors like

 ERR lib/restaurants/app/components/second-component.js Transformation error (Cannot read property 'replace' of undefined)
TypeError: Cannot read property 'replace' of undefined
    at replace (/Users/rcohen/code/spikes/ember-lib/fake-dashboard/node_modules/ember-codemods-telemetry-helpers/lib/utils/telemetry.js:46:30)
    at _generateModuleKey (/Users/rcohen/code/spikes/ember-lib/fake-dashboard/node_modules/ember-codemods-telemetry-helpers/lib/utils/telemetry.js:39:19)
  ...

that appear to be because the logic in getModulePathFor only supports converting files in the addon folder.

Would a PR be welcome that handles getting the module path for files in in-repo-addons app folders?

rwjblue commented 4 years ago

Would a PR be welcome that handles getting the module path for files in in-repo-addons app folders?

Absolutely! It will be somewhat hard to map when there are conflicts, but I think it should be possible.

raycohen commented 4 years ago

The ember-native-class-codemod package doesn't pass the input/output diff test from yarn test when I yarn link this package (clean master branch) and pass analyzeEmberObject to gatherTelemetryForUrl. Is that expected?

rwjblue commented 4 years ago

Hmm, no I'm not sure why that would be failing :thinking:

tylerturdenpants commented 4 years ago

@raycohen this is fixed in #20

rwjblue commented 4 years ago

Reopening because I think #20 fixed the existing failures (not the issue that @raycohen was reporting).

tylerturdenpants commented 4 years ago

Agreed.

tylerturdenpants commented 4 years ago

I should have been more clear in the my previous comment.

raycohen commented 4 years ago

You should know better than to use this so lightly ;)

rwjblue commented 4 years ago

LOL