Update literal paths #21
Conversation
There was a problem hiding this comment.
This is a nice addition!
I removed the beautify code. If you would like, I can add this in a different PR.
It would make sense in an other PR 😀.
I like this change. I was thinking of using Prettier to replace the beautifyImport function (scoped to the updated / added imports). But for now, it's probably better to remove the function.
To make the CI happy, you'll need to run yarn lint:js --fix.
snewcomer
force-pushed
the
sn/replace-old-paths
branch
from
7e14528 to
47d5d20
Compare
|
Updated with master! |
bin/ember-data-codemod.js
Outdated
| @@ -11,7 +11,7 @@ let cwd = process.cwd(); | |||
| let pkgPath = path.join(cwd, 'package.json'); | |||
|
|
|||
| const DEFAULT_PATHS = [ | |||
| 'app', | |||
| 'app' | |||
| 'addon', | |||
There was a problem hiding this comment.
Looks like we have a typo here
__testfixtures__/literal.output.js
Outdated
| @@ -0,0 +1,4 @@ | |||
| import Model, { attr, belongsTo, hasMany } from '@ember-data/model'; | |||
There was a problem hiding this comment.
I updated this output to show the failing test. My bad, this isn't ready to go. I ran it on our project and had to manually collapse so we didn't have duplicate imports. I'll try to fix.
There was a problem hiding this comment.
No worries, let me know when you need another review.
There was a problem hiding this comment.
@dcyriller Ready for your review! Tested on a fairly large project and worked with no eslint errors.
| * After modifying existing sources to their new paths, we need | ||
| * to make sure we clean up duplicate imports | ||
| */ | ||
| function cleanupDuplicateLiteralPaths() { |
There was a problem hiding this comment.
I wanted to use the existing registry.modules somehow, but at this point in the codemod, they might contain old information. So in this solution, I went with iterating over the imports (post updating their source) and adding to the existing import if found.
__testfixtures__/literal.input.js
Outdated
| @@ -0,0 +1,5 @@ | |||
| import Model, { attr } from 'ember-data/model'; | |||
There was a problem hiding this comment.
Here, it seems we have a wrong import. It should be:
import Model from 'ember-data/model';
import attr from 'ember-data/attr';
There was a problem hiding this comment.
It won't make the test fail, because attr is identified in the modules lookup.
|
Thank you very much @snewcomer! Do you prefer to rewrite a bit the git history or should I squash and merge? |
|
Also, I'll fill an issue to update For instance, |
|
@dcyriller Squash and merge. Also, yeah a follow up issue is something we can work on after (perhaps today)! |
|
🍻 Issue is here: #23 |
|
FYI @snewcomer I've noticed an issue regarding updating literal paths. I've opened here a PR with a failing test: #24 If you don't have time to look at it, I will have some on Friday. :) |
This PR attempts to replace examples like
ember-data/modelwith@ember-data/modelbased on the mappings here: https://github.com/ember-data/ember-data-rfc395-dataMoreover, I removed the beautify code. I'm not sure this codemod should be modifying these imports as show here - ember-codemods/ember-modules-codemod#90. If you would like, I can add this in a different PR.