8000 Use app router template by gautamsi · Pull Request #9186 · keystonejs/keystone · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Use app router template #9186

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.

Sign up for GitHub

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

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

Use app router template #9186

wants to merge 2 commits into from

Conversation

gautamsi
Copy link
Member
@gautamsi gautamsi commented Jun 25, 2024 •
edited
Loading

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

For anyone interested in testing this out, I have published temporary packages (use at your own risk) I have been using it with very large project of mine.
@k6js-next/core-next@0.0.0-20240905-01
@k6js-next/auth-next@0.0.0-20240905-01

to use this you have to specify resolutions, for yarn I do it like this in (root package.json in monorepo)

"resolutions": {
    "graphql": "16.8.1",
    "next": "14.2.5",
    "react": "^18.3.1",
    "react-dom": "^18.3.1",
    "@keystone-6/core": "npm:@k6js-next/core-next@0.0.0-20240905-01",
    "@keystone-6/auth": "npm:@k6js-next/auth-next@0.0.0-20240905-01"
  }

@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
@codesandbox-ci CodeSandbox CI
Copy link
codesandbox-ci bot commented Jun 25, 2024 •
edited
Loading

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 3e6d294:

Sandbox Source
@keystone-6/sandbox Configuration

@gautamsi
Copy link
Member Author
gautamsi commented Jun 25, 2024 •
edited
Loading

@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 •
edited
Loading

@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 14 times, most recently from 303b060 to f86b3fd Compare June 30, 2024 12:31
@gautamsi
Copy link
Member Author
gautamsi commented Jun 30, 2024 •
edited
Loading

@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.

gautamsi
gautamsi commented Jun 30, 2024
View reviewed changes
packages/core/src/admin-ui/system/generateAdminUI.ts
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

@gautamsi
Copy link
Member Author
gautamsi commented Jul 7, 2024

@dcousens do you plan to check this out soon?

@gautamsi gautamsi mentioned this pull request Jul 18, 2024
Open
@gautamsi gautamsi mentioned this pull request Jul 25, 2024
Merged
@gautamsi gautamsi changed the base branch from v-next to main February 23, 2025 05:50
@gautamsi gautamsi force-pushed the #9183#1 branch 4 times, most recently from b8119fc to 24927f2 Compare February 25, 2025 23:59
@gautamsi
Copy link
Member Author
gautamsi commented Feb 26, 2025 •
edited
Loading

@dcousens I have updated the branch from latest main, please look into this.
Let me know if this is even the direction you want to go as I see more traction from other team members

@emmatown can you help me with the failing smoke tests, they get stuck on starting and don't do anything.

@gautamsi gautamsi force-pushed the #9183#1 branch 5 times, most recently from 414e7a5 to b0925ae Compare March 2, 2025 22:04
@dcousens
Copy link
Member
dcousens commented Mar 2, 2025 •
edited
Loading

@gautamsi we should be at a point where we can talk internally about this soon - sorry for the delay, as you know we put priority into the broader changes first 💛

@gautamsi
Copy link
Member Author

any traction on this? @dcousens

@gautamsi gautamsi force-pushed the #9183#1 branch 2 times, most recently from cf90b62 to 28ba02a Compare March 30, 2025 01:06
@gautamsi gautamsi force-pushed the #9183#1 branch 2 times, most recently from 976d207 to 0f8b11d Compare April 12, 2025 04:44
@gautamsi
Copy link
Member Author

any plan to look into this @dcousens ?? I will update if you have plan, or move on if this is not in your agenda

@gautamsi gautamsi force-pushed the #9183#1 branch 2 times, most recently from 1e0a400 to 2c1e048 Compare May 18, 2025 23:36
@dcousens
Copy link
Member
dcousens commented May 18, 2025 •
edited
Loading

Sorry for the delay @gautamsi, this is not on our immediate agenda - however, we are sparring around the issue more broadly, not this pull request

Please keep the pull request open 💛🩵

@gautamsi gautamsi force-pushed the #9183#1 branch 2 times, most recently from 7712870 to f7a30e7 Compare June 19, 2025 03:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
breaking
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gautamsi @dcousens
0