Skip to content

Conversation

HeinrichvonStein
Copy link

@HeinrichvonStein HeinrichvonStein commented Nov 28, 2024

This PR adds a Tutorial section to the docs as well as a new Coolify + PowerSync integration guide.

Mintlify dev server running on an EC2 instance: server

Specific pages:

Not too sure about the videos and possible storage implications. Open to any suggestions

@benitav
Copy link
Collaborator

benitav commented Nov 28, 2024

is it correct that this is still in draft status? just checking about it since typically we update that status to review if it's ready for review.

@HeinrichvonStein HeinrichvonStein marked this pull request as ready for review November 29, 2024 05:18
@@ -0,0 +1,271 @@
---
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it would have been cool if we chose one of these two implementations as the default, but maybe it's fine to have a baseline connector example of a supabase connector and give the options to improve it with these two snippets.

Copy link
Author

Choose a reason for hiding this comment

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

We could yes. My thinking behind it was that the examples provide an easy way for users to get started and see example implementations - which are inherently basic and not performance optimized. Should they require better performance or some other improvement, they can have a look at the tutorials to see how that could be done.

It also provides an easy and concise way for users to see what needs to be changed should they want to make that specific improvement.

@benitav
Copy link
Collaborator

benitav commented Nov 29, 2024

"Code examples" isn't a very useful tutorial grouping IMO - won't all tutorials be/include code examples? I think a grouping by topic would be more intuitive e.g. backend database, client side, self hosting, because that's how users faced with a problem will likely navigate/filter. Maybe I need to better understand what other groups we are envisioning aside from code examples.

title: "Use AWS S3 for attachment storage"
description: "In this tutorial, we will show you how to use AWS S3 for attachment storage by modifying the [React Native To-Do List example app](https://github.com/powersync-ja/powersync-js/tree/main/demos/react-native-supabase-todolist)."
sidebarTitle: "AWS S3 attachment storage"
---
Copy link
Collaborator

Choose a reason for hiding this comment

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

to orient the user state what makes this different from the example app - i.e. there Supabase storage is used

Copy link
Collaborator

Choose a reason for hiding this comment

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

We mention React Native here, but will the same/similar also apply to web? If so we should state that, so that web users also know whether this could be relevant to them

Copy link
Author

@HeinrichvonStein HeinrichvonStein Dec 2, 2024

Choose a reason for hiding this comment

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

@benitav The deps I added are only applicable for React Native so I'm unsure what deps would be required for web and we would probably have a similar tutorial for web-based projects.


<Warning>
The following pre-requisites are required to complete this tutorial:
- Clone the [To-Do List example app](https://github.com/powersync-ja/powersync-js/tree/main/demos/react-native-supabase-todolist)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I find it a bit unintuitive saying cloning an app - they need to clone the repo?

```
</Accordion>

<Accordion title="Explanation">
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would keep uploadFile, downloadFile, deleteFile expanded - since it's not a lot of text the additional clicks expanding the accordions is more annoying that useful. The user already opted to see the Explanation.

</AccordionGroup>

## The complete files used in this tutorial can be found below:
<Accordion title="Files">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since the files are fairly compact already with the tabs (nice), I don't think the accordion adds a lot of value here- again, feels more annoying to have to do the extra click than useful.

<AccordionGroup>
<Accordion title="Sequential Merge Strategy">
<Note>
Shoutout to Christoffer Årstrand for the original implementation of this optimization.
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be safer to use people's username / handle - since some people might not be comfortable having their name out there

Copy link
Author

Choose a reason for hiding this comment

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

Good catch, didn't think about that. Will update it to use discord handle

title: "Improve Supabase Connector Performance"
description: "In this tutorial we will show you how to improve the performance of the Supabase Connector for the [React Native To-Do List example app](https://github.com/powersync-ja/powersync-js/tree/main/demos/react-native-supabase-todolist)."
---

Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs an intro to set the stage. All I see is various "strategy" headings, but I don't quickly have an idea what the problem is with the existing implementation, how we're going to solve it, and how all the headings fit in.

Related, it's not clear to me whether I should open any of those accordions, and after opening one I just see a big blob of code, with no explanation/intro/guidance.

Copy link
Author

Choose a reason for hiding this comment

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

Agreed, context is definitely a good thing!
Will add both context and more guidance on the code!

@benitav
Copy link
Collaborator

benitav commented Nov 29, 2024

Overall, we need to think about cross referencing to these new sections across relevant other topics in the docs. For example, think about users self-hosting and following the current Installation instructions. They might not realize that we have this Coolify tutorial, so when it becomes relevant in the process, we should say something like "using coolify? check out our guide". Another example is whether we should add to/update "Deployment Templates (15 minutes)" on the Self Hosting -> Getting started page to mention this tutorial. Another example is linking to Tutorials on our main page https://docs.powersync.com/intro/powersync-overview - possibly under Examples - then that heading could be renamed. These are just examples I could think of quickly - def not exhaustive.

@HeinrichvonStein
Copy link
Author

HeinrichvonStein commented Nov 29, 2024

"Code examples" isn't a very useful tutorial grouping IMO - won't all tutorials be/include code examples? I think a grouping by topic would be more intuitive e.g. backend database, client side, self hosting, because that's how users faced with a problem will likely navigate/filter. Maybe I need to better understand what other groups we are envisioning aside from code examples.

The tutorials might include things like "Adding PowerSync to your project" which doesn't really fit into the "Code examples" category. My thinking behind "Code examples" was that those tutorials would show how to make specific changes in the code or focus heavily on code changes rather than the typical "step by step guide". It shows the user how they could modify the code.

I definitely like the idea of more intuitive groupings as suggested.

Edit:
Alternatively, we could group the integration guides and tutorials together as two main headings from which things could branch off of?

@HeinrichvonStein HeinrichvonStein merged commit 00e4bd3 into docs Dec 4, 2024
1 check passed
@HeinrichvonStein HeinrichvonStein deleted the feat/tutorials branch December 4, 2024 06:27
@cahofmeyr
Copy link
Collaborator

@benitav should we also link to these tutorials from other places?

For example link to the S3 one from here https://docs.powersync.com/usage/use-case-examples/attachments-files

And link to the Supabase one from here https://docs.powersync.com/integration-guides/supabase-+-powersync and maybe here https://docs.powersync.com/resources/demo-apps-example-projects

@benitav
Copy link
Collaborator

benitav commented Dec 9, 2024

Yeah @HeinrichvonStein - I had similar feedback #44 (comment). Let me know if I can leave this with you?

@HeinrichvonStein
Copy link
Author

@benitav @cahofmeyr Will update the docs accordingly.

Comment on lines +66 to +69
EXPO_PUBLIC_AWS_S3_REGION=region
EXPO_PUBLIC_AWS_S3_BUCKET_NAME=bucket_name
EXPO_PUBLIC_AWS_S3_ACCESS_KEY_ID=***
EXPO_PUBLIC_AWS_S3_ACCESS_SECRET_ACCESS_KEY=***
Copy link
Contributor

Choose a reason for hiding this comment

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

This bit is quite problematic for security - the AWS credentials should never be exposed directly on the client - it could expose access to the entire S3 bucket to the user.

A good workflow with S3 works like this:

  1. Client makes an API call to the app backend, using the client credentials (e.g. a Supabase Edge Function).
  2. The backend API has the S3 credentials. It signs a S3 upload URL, and returns that to the client.
  3. The client uploads to the pre-signed S3 URL.

This workflow keeps the credentials on the server, without the actual attachment data needing to go through the server.

The same can be done for downloading attachments.

The Uppy has some good documentation on uploading to S3 from a client.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @rkistner We'll take a look.

Copy link
Author

Choose a reason for hiding this comment

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

@michaelbarnes @rkistner I've created #99 for this update.

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.

6 participants