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

Blitz recipe-install support for NextJS App Router projects #4287

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Doc0x1
Copy link
Contributor

@Doc0x1 Doc0x1 commented Jan 22, 2024

What are the changes and their implications?

Bringing the Blitz recipe-install feature up to speed with the newer NextJS App Router projects.

TODO:

  • Change paths.ts so that it can check and return proper App Router paths/files. There's no _app file in App Router projects so as of the initial commit, it is set to return the root layout file of the project.
  • Add in error handling for cases where folders may not exist in a NextJS App Router filesystem

Closes: #4189

Feature Checklist


Just a heads up, this is my first time contributing to a community project on GitHub, and so there's been a lot of new things I've been picking up in a pretty short amount of time. I've done my best to follow the contributing guidelines and if I've made any mistakes, feel free to let me know so I can avoid them in the future.

Anyway, this pull request is by no means ready for a merge, but I wanted to open it so I could get some feedback from those of you that might be more familiar with the recipe system than I currently am.

@Doc0x1 Doc0x1 requested a review from flybayer as a code owner January 22, 2024 18:08
Copy link

changeset-bot bot commented Jan 22, 2024

🦋 Changeset detected

Latest commit: 019bf27

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

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

@Doc0x1
Copy link
Contributor Author

Doc0x1 commented Jan 22, 2024

I think the logical next step would be adding better error handling to prevent it from hanging during a recipe install. Having it create non-existent folders is one idea I've had.

Looking over it, the issue happens at the .addTransformFilesStep part of the recipe, at least for the tailwind recipe. Working on a way to implement better error handling in the transform.ts file so it doesn't hang.

@siddhsuresh siddhsuresh requested review from siddhsuresh and removed request for flybayer January 22, 2024 19:09
@siddhsuresh
Copy link
Member

I think the logical next step would be adding better error handling to prevent it from hanging during a recipe install. Having it create non-existent folders is one idea I've had.

Looking over it, the issue happens at the .addTransformFilesStep part of the recipe, at least for the tailwind recipe. Working on a way to implement better error handling in the transform.ts file so it doesn't hang.

yeah @Doc0x1 that's a good idea. Let me know when the PR is ready for a review!

@Doc0x1
Copy link
Contributor Author

Doc0x1 commented Jan 26, 2024

I think the logical next step would be adding better error handling to prevent it from hanging during a recipe install. Having it create non-existent folders is one idea I've had.

Looking over it, the issue happens at the .addTransformFilesStep part of the recipe, at least for the tailwind recipe. Working on a way to implement better error handling in the transform.ts file so it doesn't hang.

yeah @Doc0x1 that's a good idea. Let me know when the PR is ready for a review!

Will do. I just got some good web dev work so its gonna be maybe a couple weeks before im able to get back to actively working on this, so just a heads up. Glad i could help with getting that other PR done though

@tordans tordans mentioned this pull request Jan 27, 2024
22 tasks
Comment on lines +3 to +5
content: [
"./{src/pages,src/components,src/app,src,pages,components,app}/**/*.{js,ts,jsx,tsx,mdx}",
],
Copy link
Member

Choose a reason for hiding this comment

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

🍰 https://nextjs.org/docs/app/building-your-application/styling/tailwind-css#configuring-tailwind

Suggested change
content: [
"./{src/pages,src/components,src/app,src,pages,components,app}/**/*.{js,ts,jsx,tsx,mdx}",
],
content: [
'./app/**/*.{js,ts,jsx,tsx,mdx}',
'./pages/**/*.{js,ts,jsx,tsx,mdx}',
'./components/**/*.{js,ts,jsx,tsx,mdx}',
'./src/**/*.{js,ts,jsx,tsx,mdx}',
],

@justinbalaguer
Copy link

up

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

Successfully merging this pull request may close these issues.

next-ui recipe stuck "generating file diff"
4 participants