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

Added codemod support for legacy projects with a /src folder #4290

Merged
merged 3 commits into from
Jan 25, 2024

Conversation

Doc0x1
Copy link
Contributor

@Doc0x1 Doc0x1 commented Jan 24, 2024

What are the changes and their implications?

Changes the codemod upgrade-legacy tool to add support for legacy projects which have folders nested inside a /src directory

Bug Checklist

  • Changeset added (run pnpm changeset in the root directory)

Feature Checklist

  • Changeset added (run pnpm changeset in the root directory)

Copy link

changeset-bot bot commented Jan 24, 2024

🦋 Changeset detected

Latest commit: d9be9f3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages
Name Type
@blitzjs/codemod Patch
@blitzjs/auth Patch
@blitzjs/next Patch
@blitzjs/rpc Patch
@blitzjs/config Patch
@blitzjs/generator Patch
next-blitz-auth Patch
blitz Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

…actually calling the function and added path.resolve's to findPagesDirectory
@siddhsuresh siddhsuresh requested review from siddhsuresh and removed request for flybayer January 24, 2024 05:09
Copy link
Member

@siddhsuresh siddhsuresh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the PR @Doc0x1! this is a good start.

  1. const appDir = path.resolve("app")

I feel the above line we should make the following change

- const appDir = path.resolve("app")
+ const appDir = fs.existsSync(path.resolve("app")) ? fs.existsSync(path.resolve("src"))

since in this case the user has the files only in src directory and not in the app

  1. getAllFiles(path.resolve("pages"), [], [], [".ts", ".tsx", ".js", ".jsx"]).forEach((file) => {

we should use the same utility here.

other than these, looks good!

@Doc0x1
Copy link
Contributor Author

Doc0x1 commented Jan 24, 2024

Okay @siddhsuresh sounds good, I'll implement those changes once I'm back home

@Doc0x1
Copy link
Contributor Author

Doc0x1 commented Jan 25, 2024

@siddhsuresh Okay, should be good to go for review. Let me know if there's anything else. Also I decided to throw an Error around line 49 inside a try catch statement in the event that appDir is just an empty string to add a bit of error handling towards the start.

Copy link
Member

@siddhsuresh siddhsuresh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@siddhsuresh siddhsuresh merged commit f25aac0 into blitz-js:main Jan 25, 2024
29 checks passed
@blitzjs-bot
Copy link
Contributor

Added @Doc0x1 contributions for doc and code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants