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

WIP: Addition of custom collection functions for importDeclarations/Specifiers #611

Merged
merged 1 commit into from
Jul 29, 2024

Conversation

danieldelcore
Copy link

@danieldelcore danieldelcore commented Jul 24, 2024

Adds utility methods for import specifiers (similar to the ones provided for JSXElements).

Why?

Import manipulation is probably the most common operation you will do in a codemod, but there are quite a few edge cases to consider when doing so.

For example,

  • Insert imports / specifiers without duplication
  • Renaming import paths
  • Checking for imports (good for early exit conditions)

GLOBALS

insertImportDeclaration
insertImportSpecifiers
findImportDeclarations
findImportSpecifier
findDefaultSpecifier
hasImportSpecifier
hasDefaultImportSpecifier

FILTERS

TBD

Copy link

vercel bot commented Jul 24, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
jscodeshift ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 24, 2024 8:22am

@Daniel15
Copy link
Member

This is really useful! I've written code like this so many times. Nice work.

Can this handle Flow/TypeScript type imports too?

@danieldelcore
Copy link
Author

danieldelcore commented Jul 25, 2024

Thanks @Daniel15!

I've also written code like this a lot 😂 and I find it's normally the first util people make, but given the way jscodeshift's API is designed (via dependency injection, rather than typical imports) it can be quite cumbersome to import and use utils unless you have access to the register method.

I have a util library here, which I've based most of this work on, and as you can probably guess the API is less than ideal because it's forcing me to pass the core API and AST into every method:
https://github.com/hypermod-io/hypermod-community/blob/main/packages/utils/src/imports.ts

I'll add some test cases for flow/typescript, but it should work because these utils operate on the ImportSpecifier which TS/Flow type imports are represented by AFAIK.

image

@Daniel15
Copy link
Member

Daniel15 commented Jul 29, 2024

I'm going to merge this as-is. We can add tests for type imports as a followup.

Flow uses a slightly different format for type imports, which we should test too:

import type {Foo} from 'whatever';

@Daniel15 Daniel15 merged commit 80d46fa into facebook:main Jul 29, 2024
3 checks passed
@danieldelcore
Copy link
Author

Thanks @Daniel15 I'll come back to this asap. I have more ready to push up :)

@danieldelcore
Copy link
Author

@mohab-sameh would be good to have these documented on the new site if you have time for it!

@mohab-sameh
Copy link
Collaborator

@danieldelcore sure thing! thanks for notifying me.

@mohab-sameh mohab-sameh added the Add To Docs Add to the documentation, either in the form of a README.md updated, a wiki entry, or cookbook entry label Jul 29, 2024
@Daniel15
Copy link
Member

Daniel15 commented Jul 29, 2024

I'll push a new version of jscodeshift this week, depending on when you're planning on submitting more PRs this week.

@Daniel15
Copy link
Member

Daniel15 commented Jul 30, 2024

@danieldelcore Can you please look at the test failure? Not sure why it didn't appear on this diff.

Did I merge this too early? We should ensure that PRs that aren't ready to merge are labelled as set as "drafts"

Edit: Sorry I messed this up a bit :) Please submit a new PR in draft status.

Run yarn test
yarn run v1.22.22
$ jest --bail
PASS src/collections/__tests__/Node-test.js
PASS src/__tests__/Collection-test.js
PASS src/__tests__/argsParser-test.js
FAIL src/__tests__/template-test.js
  ● Templates › interpolates expression nodes with source code

    ReferenceError: filterMethods is not defined

      112 |
      113 | exports.register = once(register);
    > 114 | exports.filters = filterMethods;
          |                   ^
      115 |

      at Object.filterMethods (src/collections/ImportDeclaration.js:114:19)
      at Object.require (src/collections/index.js:13:22)
      at Object.require (src/core.js:12:21)
      at Object.require (src/__tests__/template-test.js:23:19)

  ● Templates › interpolates statement nodes with source code

    ReferenceError: filterMethods is not defined

      112 |
      113 | exports.register = once(register);
    > 114 | exports.filters = filterMethods;
          |                   ^
      115 |

      at Object.filterMethods (src/collections/ImportDeclaration.js:114:19)
      at Object.require (src/collections/index.js:13:22)
      at Object.require (src/core.js:12:21)
      at Object.require (src/__tests__/template-test.js:23:19)

  ● Templates › can be used with a different parser

    ReferenceError: filterMethods is not defined

      112 |
      113 | exports.register = once(register);
    > 114 | exports.filters = filterMethods;
          |                   ^
      115 |

      at Object.filterMethods (src/collections/ImportDeclaration.js:114:19)
      at Object.require (src/collections/index.js:13:22)
      at Object.require (src/core.js:12:21)
      at Object.require (src/__tests__/template-test.js:23:19)

  ● Templates › handles out-of-order traversal

    ReferenceError: filterMethods is not defined

      112 |
      113 | exports.register = once(register);
    > 114 | exports.filters = filterMethods;
          |                   ^
      115 |

      at Object.filterMethods (src/collections/ImportDeclaration.js:114:19)
      at Object.require (src/collections/index.js:13:22)
      at Object.require (src/core.js:12:21)
      at Object.require (src/__tests__/template-test.js:23:19)

  ● Templates › correctly parses expressions without any interpolation

    ReferenceError: filterMethods is not defined

      112 |
      113 | exports.register = once(register);
    > 114 | exports.filters = filterMethods;
          |                   ^
      115 |

      at Object.filterMethods (src/collections/ImportDeclaration.js:114:19)
      at Object.require (src/collections/index.js:13:22)
      at Object.require (src/core.js:12:21)
      at Object.require (src/__tests__/template-test.js:23:19)

  ● Templates › asyncExpression correctly parses expressions with await -- babel

    ReferenceError: filterMethods is not defined

      112 |
      113 | exports.register = once(register);
    > 114 | exports.filters = filterMethods;
          |                   ^
      115 |

      at Object.filterMethods (src/collections/ImportDeclaration.js:114:19)
      at Object.require (src/collections/index.js:13:22)
      at Object.require (src/core.js:12:21)
      at Object.require (src/__tests__/template-test.js:23:19)

      112 |
      113 | exports.register = once(register);
    > 114 | exports.filters = filterMethods;
          |                   ^
      115 |

      at Object.filterMethods (src/collections/ImportDeclaration.js:114:19)
      at Object.require (src/collections/index.js:13:22)
      at Object.require (src/core.js:12:21)
      at Object.require (src/__tests__/template-test.js:23:19)

  ● Templates › explode arrays › explodes arrays in function definitions

    ReferenceError: filterMethods is not defined

      112 |
      113 | exports.register = once(register);
    > 114 | exports.filters = filterMethods;
          |                   ^
      115 |

      at Object.filterMethods (src/collections/ImportDeclaration.js:114:19)
      at Object.require (src/collections/index.js:13:22)
      at Object.require (src/core.js:12:21)
      at Object.require (src/__tests__/template-test.js:23:19)

  ● Templates › explode arrays › explodes arrays in variable declarations

    ReferenceError: filterMethods is not defined

      112 |
      113 | exports.register = once(register);
    > 114 | exports.filters = filterMethods;
          |                   ^
      115 |

      at Object.filterMethods (src/collections/ImportDeclaration.js:114:19)
      at Object.require (src/collections/index.js:13:22)
      at Object.require (src/core.js:12:21)
      at Object.require (src/__tests__/template-test.js:23:19)

  ● Templates › explode arrays › explodes arrays in array expressions

    ReferenceError: filterMethods is not defined

      112 |
      113 | exports.register = once(register);
    > 114 | exports.filters = filterMethods;
          |                   ^
      115 |

      at Object.filterMethods (src/collections/ImportDeclaration.js:114:19)
      at Object.require (src/collections/index.js:13:22)
      at Object.require (src/core.js:12:21)
      at Object.require (src/__tests__/template-test.js:23:19)

  ● Templates › explode arrays › explodes arrays in object expressions

    ReferenceError: filterMethods is not defined

      112 |
      113 | exports.register = once(register);
    > 114 | exports.filters = filterMethods;
          |                   ^
      115 |

      at Object.filterMethods (src/collections/ImportDeclaration.js:114:19)
      at Object.require (src/collections/index.js:13:22)
      at Object.require (src/core.js:12:21)
      at Object.require (src/__tests__/template-test.js:23:19)

  ● Templates › explode arrays › explodes arrays in call expressions

    ReferenceError: filterMethods is not defined

      112 |
      113 | exports.register = once(register);
    > 114 | exports.filters = filterMethods;
          |                   ^
      115 |

      at Object.filterMethods (src/collections/ImportDeclaration.js:114:19)
      at Object.require (src/collections/index.js:13:22)
      at Object.require (src/core.js:12:21)
      at Object.require (src/__tests__/template-test.js:23:19)

Test Suites: 1 failed, 3 passed, 4 of 16 total
Tests:       15 failed, 80 passed, 95 total
Snapshots:   0 total
Time:        4.19 s
Ran all test suites.
error Command failed with exit code 1.

@danieldelcore
Copy link
Author

Oh, no worries; yeah, it was a bit early, but I'm happy to reopen :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Add To Docs Add to the documentation, either in the form of a README.md updated, a wiki entry, or cookbook entry CLA Signed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants