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

Feature: Support AWS SDK JS v3 #22

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

Conversation

anthlasserre
Copy link

@anthlasserre anthlasserre commented May 30, 2024

Context

I have been looking for an active Cloudflare R2 plugin for Strapi but I couldn't. So I have decided to re-work it from scratch. Here are the references I have been using to generate this new index.js file.

Implementation

  • Support of private buckets
  • Image Preview within Media Library admin
  • Readme update

Issue related

Here is an issue related to this PR.

@JannikZed JannikZed requested a review from tilman May 31, 2024 09:35
Copy link
Contributor

@tilman tilman left a comment

Choose a reason for hiding this comment

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

Hey, first of all thanks for your work and your PR.

Unfortunately I can not merge it in the current state. Seems like the whole functionality of using R2 with your own domain is getting removed with this PR (config.cloudflarePublicAccessUrl). This is a crucial feature and I think for many people the reason they are using this provider instead of the default strapi aws provider (which can be used with R2 as well). Please add this feature to your PR before we can merge it into main.

Thanks a lot and let me know if you have any further questions.

@anthlasserre
Copy link
Author

Hey, first of all thanks for your work and your PR.

Unfortunately I can not merge it in the current state. Seems like the whole functionality of using R2 with your own domain is getting removed with this PR (config.cloudflarePublicAccessUrl). This is a crucial feature and I think for many people the reason they are using this provider instead of the default strapi aws provider (which can be used with R2 as well). Please add this feature to your PR before we can merge it into main.

Thanks a lot and let me know if you have any further questions.

Hey 👋🏼
Can you please let me know what's the difference with using the endpoint param?

@anthlasserre
Copy link
Author

🆙 @tilman

@JannikZed
Copy link
Contributor

@anthlasserre the endpoint is the actual S3 endpoint of the bucket.
If you want media to be publicly available, you will need to use a publicAccessUrl URL for that. I marked you here the cloudflarePublicAccessUrl and the S3 endpoint:
Screenshot 2024-06-10 at 23 37 13

@anthlasserre
Copy link
Author

Thanks for your answer @JannikZed. I'll give a try with a public bucket and I'll let you know

@anthlasserre
Copy link
Author

@JannikZed Can you please let me know if it's fine for you now ?

);
});
},
file.url = getFileURL(file, config);
Copy link

@Dhwanil-Shah Dhwanil-Shah Jul 11, 2024

Choose a reason for hiding this comment

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

This should not assign back into file.url, the getFileURL is already doing that and causes this to overwrite and make it null, causing rendering issues for the file in the dashboard.

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.

4 participants