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

Use app router template #9186

Open
wants to merge 18 commits into
base: v-next
Choose a base branch
from
Open

Use app router template #9186

wants to merge 18 commits into from

Conversation

gautamsi
Copy link
Member

@gautamsi gautamsi commented Jun 25, 2024

ref #9183

Blocked by

This PR updates the template to generate the App router, default to typescript version which can be turned off by setting config.ui.tsx = false

What is completed/Verified:

  • generate app routes in app/(admin) folder (creates admin route specific layout page. if src page exist, it generates it in `src/app/(admin)
  • Stop generating next.config.js file, this should be generated from create-keystone-app going forward (TODO)
  • set default ui.basePath to /admin can set ui.basePath to /admin or any other sub path
  • No longer copies admin files or generates pages export copy as they are no longer needed
  • Does not clear the admin folder files if exist, only overwrites the special config files which contains up to date admin meta and view cache, this should be removed before GA
  • Verified custom pages
  • Verified custom fields and field views
  • can use relative path or even "paths" from typescript (@fields/text/myView)
  • Added --reset-admin flag to dev command to force clean (admin) folder, useful in regenerate the admin template if updated in future. should never be needed except braking upgrade
  • fix build and start script
  • @keystone-6/auth templates
  • page middleware

Pending:

  • Documentation
  • fixing final set of failing tests

@dcousens can you create a v-next branch which should be actual target for these PRs instead of main

@gautamsi gautamsi force-pushed the #9183#1 branch 4 times, most recently from 29477ea to 19ab9c5 Compare June 25, 2024 06:08
@dcousens dcousens changed the base branch from main to v-next June 25, 2024 06:31
Copy link

codesandbox-ci bot commented Jun 25, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit f86b3fd:

Sandbox Source
@keystone-6/sandbox Configuration

@gautamsi
Copy link
Member Author

gautamsi commented Jun 25, 2024

@dcousens I will need some help in fixing tests,
* smoke tests never finishes
* wired error in package unit tests, it has mismatch on keystone error message in build but unable to trace.
* random test failure with checkForTooManyEngines prisma error

I have disabled some ui tests, they need fixes but we can do that before merging to main

@dcousens I have fixed most test cases, these tests definitely helped me fix a lot of issues.

I see that document-field test can not be fixed due to static generation, I may have missed something.

Test live-reloading.test is not behaving properly, same thing, trying to fix some page router vs app router thing. I am having trouble debugging locally on windows for some reason.

@dcousens
Copy link
Member

dcousens commented Jun 25, 2024

@gautamsi I'm super busy and can't engage on this this week, but, I'll wrap back around as soon as I can and help move this forward 💛

@gautamsi gautamsi force-pushed the #9183#1 branch 2 times, most recently from f4cf9b7 to f70d1fe Compare June 30, 2024 05:46
@gautamsi gautamsi force-pushed the #9183#1 branch 2 times, most recently from 3794b1b to 7f16a79 Compare June 30, 2024 07:49
@gautamsi gautamsi force-pushed the #9183#1 branch 6 times, most recently from 4a81dc4 to 00a6767 Compare June 30, 2024 10:58
@gautamsi gautamsi force-pushed the #9183#1 branch 2 times, most recently from e075014 to 303b060 Compare June 30, 2024 12:30
@gautamsi
Copy link
Member Author

gautamsi commented Jun 30, 2024

@dcousens I have not generated default files for all the example/test projects, have modified the code as such it will generate all the required nextjs files for dev and build. User would need to commit them in git.

The file generation is one time except the .admin/index file which generates the admin meta has and static keystone props for admin.

isLiveReload: boolean
) {
// when we're not doing a live reload, we want to clear everything out except the .next directory (not the .next directory because it has caches)
// so that at least every so often, we'll clear out anything that the deleting we do during live reloads doesn't (should just be directories)
if (!isLiveReload) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This part of live reload is not needed, we do not even need to figure out file linking from main dir to isolated dir anymore

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

Successfully merging this pull request may close these issues.

None yet

2 participants