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

Closed
wants to merge 44 commits into 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: 0289d65

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 and others added 2 commits January 25, 2024 14:18
…s#4290)

* Added support for legacy projects that have the pages folder located inside a /src directory

* Fixed line 687 for path.resolve(`${findPagesDirectory}/api/rpc`) not actually calling the function and added path.resolve's to findPagesDirectory

* Update for lines 56 and 1340 to add support for app and pages directories located in /src
@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}',
],

siddhsuresh and others added 16 commits February 6, 2024 16:13
* perf: add check to make sure only non expired sessions are selected by default

* remove console.logs

* Create chatty-ants-bake.md

* remove `expiresAt` from the publicData

* remove internal from changelog

---------

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
* feat: export `BlitzServerMiddleware` from blitz-next

* Create sixty-pants-hunt.md

---------

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
* add ajv to devDeps

* Create four-dots-retire.md
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
… Nextjs 13 or higher (blitz-js#4299)

* fix: remove custom errors thrown by blitz

* Create curvy-cougars-lick.md

* use require and eval rather than the await which becomes a `yield import` possibly causing the issue

* pnpm lock fix

* Update .changeset/curvy-cougars-lick.md

* add comment

* use correct error type

* Apply suggestions from code review
…#4300)

* feat: remove restriction to use secure cookies in localhost

* changeset

* pnpm lock fix

* Update .changeset/grumpy-deers-rest.md

* Update .changeset/grumpy-deers-rest.md

---------

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
* Version Packages

* pnpm lock

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Siddharth Suresh <[email protected]>
* fix: process.kill("SIGABRT") not supported on windows

* fix: use process.kill("SIGINT") across all operative systems

* Update .changeset/clever-insects-shave.md

---------

Co-authored-by: Siddharth Suresh <[email protected]>
* fix: production issue

* Update .changeset/soft-tables-ring.md

---------

Co-authored-by: Siddharth Suresh <[email protected]>
* Version Packages

* chore: update pnpm lock

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Siddharth Suresh <[email protected]>
…blitz-js#4318)

* fix stray DYNAMIC_SERVER_USAGE thrown

* cleanup

* chore: changeset
timneutkens and others added 7 commits April 5, 2024 10:28
* Use this.rootContext instead of webpack internals

Ensures the root context is read from the public API that webpack exposes. This is the first step for Turbopack support as Turbopack includes `this.rootContext` as well

* Turbopack support for Blitz

* Update packages/blitz-rpc/src/server/loader/server/loader-server.ts

Co-authored-by: Tobias Koppers <[email protected]>

* fix: CI and update next.js version in test app

* feat: add tests for turbo and expose new `turbo` boolean

* upgrade to latest next version

* use latest canary in internal packages and tests

* chore: add changeset

* chore: minor fix on how the test is run

* fix stray DYNAMIC_SERVER_USAGE thrown

* cleanup

* chore: changeset

* pnpm lock fix

* fix turbo tests

* fixes

* oops

* add turbo config only when needed

* remove need for any change to `next.config.js`

---------

Co-authored-by: Tobias Koppers <[email protected]>
Co-authored-by: Siddharth Suresh <[email protected]>
* Version Packages

* pnpm lock fix

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Siddharth Suresh <[email protected]>
* fix: add missing host in next-auth adapter

* Create red-masks-drop.md

* Update .changeset/red-masks-drop.md
* do not oveeride existing config

* Create calm-deers-sin.md

* Update packages/blitz-next/src/index-server.ts
* Version Packages

* update pnpm lock

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Siddharth Suresh <[email protected]>
@justinbalaguer
Copy link

up

siddhsuresh and others added 6 commits May 9, 2024 14:05
* move it back to index-server

* fix turbopack

* Create big-cars-raise.md

* cleanup

* fix

* fix lint
* Version Packages

* pnpm lock fix

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Siddharth Suresh <[email protected]>
* upgrade to latest v4

* Create warm-scissors-juggle.md

* Update .changeset/warm-scissors-juggle.md

* also check for flightcontrol platform
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@mvarchdev
Copy link

I do not know what is the state, but I tried blitz and I have just issues, this is one of them... ERR_INVALID_ARG_TYPE, The "path" argument must be of type string or an instance of Buffer or URL. Received undefined, probably does soemthing with this

@JWBrownie
Copy link

I am still having this issue as of for version 2.0.10 is it safe to use the recipe even if it errors out?

Thanks,

@Doc0x1
Copy link
Contributor Author

Doc0x1 commented Jun 27, 2024

Might need to close this one, was working on updating it for the current upstream main branch of Blitz and not sure if I did so correctly.

@siddhsuresh
Copy link
Member

Hey, I am closing this for now, feel free to reopen it and request a re-review when possible. Thanks!

@Doc0x1
Copy link
Contributor Author

Doc0x1 commented Oct 11, 2024

Hey, I am closing this for now, feel free to reopen it and request a re-review when possible. Thanks!

Yeah no problem, I actually need to look through the code again and get a better understanding of how the generator functions before finishing it up in the future.

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"