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

chore: add docs about file upload #7338

Closed
wants to merge 3 commits into from

Conversation

bartoszherba
Copy link
Collaborator

@bartoszherba bartoszherba commented Nov 29, 2024

closing: ES-1361

Copy link

changeset-bot bot commented Nov 29, 2024

⚠️ No Changeset found

Latest commit: 34bd632

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Comment on lines 138 to 142
export async function uploadFile(
context: IntegrationContext
): Promise<CustomMethodResponse> {
const files = context.req.files;
const additionalParams = context.req.body; // Access additional parameters
Copy link
Contributor

Choose a reason for hiding this comment

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

this won't work?

Suggested change
export async function uploadFile(
context: IntegrationContext
): Promise<CustomMethodResponse> {
const files = context.req.files;
const additionalParams = context.req.body; // Access additional parameters
export async function uploadFile(
context: IntegrationContext,
additionalParams: any,
): Promise<CustomMethodResponse> {
const files = context.req.files;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Why? This is exactly what I have in my poc and it does work.

export async function uploadFile(context: IntegrationContext): Promise<CustomMethodResponse> {
  const files = context.req.files;
  const params = context.req.body;

  console.log("params", params);
  console.log(params.test);
  if (!files) {
    throw new Error("No files were uploaded");
  }

  const formData = new FormData();
  // Handle both multiple files and single file
  if (Array.isArray(files)) {
    files.forEach((file) => {
      // Multer file objects contain buffer and originalname
      formData.append("files", new Blob([file.buffer]), file.originalname);
    });
  }

  return "well done";
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Your approach also works and I think it is a better way to do it. I will update the example

All additional parameters will be automatically included in the multipart/form-data request. You can include any serializable data alongside your files.
::

## Framework-Specific Examples
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can remove this section. There is nothing Alokai-specific here, so it's not worth to maintain such docs. (also the Next.js part looks quite weird to me)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If you think it is not needed then I will remove it. Also, I am not sure what is weird as I do not have that much experience in react to write non-weird stuff, whatever it is.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did that section to have a real world example.

Copy link
Contributor

Choose a reason for hiding this comment

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

sure, I don't blame you Bartosz :D I suggest removing it, if users will struggle with implementing file input form, we can think again of adding it, but I don't think that we have capacity and ambition to learn customers React / Vue. I think its fair to assume that if we show customers how to use the newly created method to upload file, they will be capable to implement the remaining ui part.

And regarding the "weird Next.js part":

  1. For Next.js we use now the App router and here is how to create a handler https://nextjs.org/docs/app/building-your-application/routing/route-handlers
  2. But anyway, there is no need to create it - we could just put this logic inside a React component

Comment on lines 219 to 223
## Usage with Middleware

::success
To read more about how to use file upload with the Middleware, please refer to the [File Upload Middleware Guide](https://docs.alokai.com/middleware/guides/file-upload).
::
Copy link
Contributor

@jagoral jagoral Nov 29, 2024

Choose a reason for hiding this comment

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

If we want to split the guides into two places (middleware and sdk pages), I would move this reference to the top of the docs, as the middleware part has to be implemented first. But imo it would be better to have everything described in one place.. (but then - which one? - idk, maybe the Storefront docs?)

Copy link
Collaborator Author

@bartoszherba bartoszherba Nov 30, 2024

Choose a reason for hiding this comment

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

I think that there are 2 places that we can use:

  1. general guides https://docs.alokai.com/guides - i think that these guides are more general than the upload file problem
  2. storefronts https://docs.alokai.com/storefront - this might be a best spot

I think that storefronts might be the best place.
Once we go through this CR iteration I will merge both md into one and I will put them in the storefront docs

```typescript
const formData = new FormData();
formData.append("files", fileInput.files[0]);
formData.append("userId", "12345");
Copy link
Contributor

Choose a reason for hiding this comment

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

@bartoszherba did this get updated? I recall I couldn't pass formData from the frontend via the SDK. Cool if it does work.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i removed that

@bartoszherba
Copy link
Collaborator Author

@jagoral @rohrig
I have moved docs to one page in storefront docs => https://github.com/vuestorefront/unified-storefronts/pull/1845

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.

3 participants