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

Update FormData and route handlers examples #179

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

Conversation

max-programming
Copy link
Contributor

@max-programming max-programming commented Sep 6, 2024

This PR updates the FormData and Route Handlers examples

  • nextjs-upload-formdata demo will be

    • generating the signature using API route/route handlers
    • uploading the image from the frontend
  • nextjs-route-handlers-upload demo will be

    • Sending the image in FormData format to the route handler (app router) or api route (pages router)

Some breaking changes

Environment variables are as follows now (update in deployment - if deployed)

nextjs-upload-formdata

NEXT_PUBLIC_CLOUDINARY_CLOUD_NAME=""
NEXT_PUBLIC_CLOUDINARY_API_KEY=""
NEXT_PUBLIC_CLOUDINARY_UPLOADS_FOLDER=""

CLOUDINARY_API_SECRET=""

nextjs-route-handlers-upload

CLOUDINARY_CLOUD_NAME=""
CLOUDINARY_API_KEY=""
CLOUDINARY_UPLOADS_FOLDER=""
CLOUDINARY_API_SECRET=""

Copy link

vercel bot commented Sep 6, 2024

@max-programming is attempting to deploy a commit to the Cloudinary DevX Team on Vercel.

A member of the Team first needs to authorize it.

@max-programming max-programming marked this pull request as ready for review September 15, 2024 07:35
@colbyfayock
Copy link
Collaborator

hey is this still a WIP per your description?

@max-programming
Copy link
Contributor Author

max-programming commented Sep 19, 2024

@colbyfayock Yes, the README updates are remaining. Done. Review and let me know

Plus I'll try to update the rest of 2 remaining examples as well in this PR itself

@@ -0,0 +1,4 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

have you thought about moving prettier to the root of the project?

that way we dont have to manage this in every single repo, it would be a bit cleaner and less to worry about in each example and inherit from the root of the project

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed there is some inconsistency between prettier configs in other projects. Like svelte, vue, etc
I will take the commons from all projects using prettier and put that in the root, and other settings like prettier-plugin-svelte will keep them in the individual files

Does that work?

- Add environment variables to a `.env.local` file:
```sh
CLOUDINARY_CLOUD_NAME=""
Copy link
Collaborator

Choose a reason for hiding this comment

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

these shoulld have the NEXT_PUBLIC_ prefix onthe first 2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These variables aren't used in client side, so I removed NEXT_PUBLIC_ from them

CLOUDINARY_CLOUD_NAME=""
CLOUDINARY_API_KEY=""
CLOUDINARY_UPLOADS_FOLDER=""
Copy link
Collaborator

Choose a reason for hiding this comment

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

it looks like throughout the examples you reference differently CLOUDINARY_UPLOADS_FOLDER vs NEXT_PUBLIC_CLOUDINARY_UPLOADS_FOLDER

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason I didn't put NEXT_PUBLIC_ in these is because these env variables are not used on the client side, so it's better if they are not exposed

@colbyfayock
Copy link
Collaborator

tested both and seem to work well. push merging main to get rid of conflicts

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.

2 participants