keystone icon indicating copy to clipboard operation
keystone copied to clipboard

Add file upload in nextjs-keystone example

Open marekryb opened this issue 2 years ago • 7 comments
trafficstars

Fixes https://github.com/keystonejs/keystone/issues/8176

marekryb avatar Dec 18 '22 08:12 marekryb

🦋 Changeset detected

Latest commit: c8779bb8b52ceaa02a4a7b16537c6341a2241dc4

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

This PR includes changesets to release 1 package
Name Type
@keystone-6/example-nextjs-keystone Minor

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

changeset-bot[bot] avatar Dec 18 '22 08:12 changeset-bot[bot]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
keystone-next-docs ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Jan 28, 2023 at 0:58AM (UTC)

vercel[bot] avatar Dec 18 '22 08:12 vercel[bot]

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 58ab9a6fe6911886f1158adae813bab7b69c13fd:

Sandbox Source
@keystone-6/sandbox Configuration

codesandbox-ci[bot] avatar Dec 18 '22 08:12 codesandbox-ci[bot]

@flexdinesh There is one unsolved issue. In keystone.ts / generateUrl there is hardcoded port 4000. This will work fine for 'yarn next:dev', but will not work for 'yarn keystone:dev' (port 3000). Normally I would pass this as some env variable, but not sure how to do this in this example without complicating things. Some guidance please.

marekryb avatar Dec 18 '22 08:12 marekryb

@marekryb re: generateUrl there is hardcoded port 4000

That's a tricky one. I think we could use environment variables to identify which server we're running (keystone or next). Perhaps we could replace the current next:dev script with "next:dev": "PORT=4000 next dev" and use the PORT env variable to conditionally setup the localhost URL.

But this probably adds an unwarranted layer of complexity to a simple example. Maybe we should just create a separate example to showcase file uploads with nextjs-keystone example and leave this example be as simple as it could be.

flexdinesh avatar Jan 23 '23 03:01 flexdinesh

@flexdinesh I added passing public url through env variable (through cross-env as otherwise someone will come and complain that it doesn't work under Win). Vercel url is prioritized.

See how you like the changes. Alternatives are:

  1. split this to separate example,
  2. have two graphql apis - one plain and one with upload (upload one can be commented out),
  3. leave 'processRequest' block as commented alternative and don't change schema at all.

PS. This example does not work well at the moment due to https://github.com/keystonejs/keystone/pull/8278 (unrelated) ;-)

PS2. Currently there is some random lint error when building through next:build (but not keystone:build). It happens also on main branch (with no changes). Not sure what is happening...

marekryb avatar Jan 28 '23 13:01 marekryb

@borisno2 this fixes the issue with file uploads and graphql-yoga for me.

If someone imports keystone in a different project (workspace setup), you need to make sure to export the processRequest from the keystone project and not adding graphql-upload to the dependencies of the other project (because of instanceof checks in graphql-upload).

mmachatschek avatar May 15 '23 06:05 mmachatschek