-
Notifications
You must be signed in to change notification settings - Fork 228
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
New Storage docs #902
New Storage docs #902
Conversation
View Vercel preview at instant-www-js-storage-docs-jsv.vercel.app. |
client/www/pages/docs/storage.md
Outdated
|
||
Storage is still in **beta**, but you can request access [here](https://docs.google.com/forms/d/e/1FAIpQLSdzInffrNrsYaamtH_BUe917EOpcOq2k8RWcGM19XepJR6ivQ/viewform?usp=sf_link)! | ||
Here's a full example of how to upload and display a grid of images |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: perhaps we add an intro sentence:
"Let's start with a fresh NextJS app:" or something like this?
client/www/pages/docs/storage.md
Outdated
|
||
// Types | ||
// ---------- | ||
export type Image = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: if we are using schema
, i.m.o we can just extract the type from it.
For example:
type InstantFile = InstaQLEntity<AppSchema, '$files'>
client/www/pages/docs/storage.md
Outdated
}); | ||
|
||
if (isLoading) { | ||
return <div>Fetching data...</div>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I would return null
for isLoading.
Reasoning: Otherwise this will have a bit of a jarring flash when you first load the page
client/www/pages/docs/storage.md
Outdated
|
||
// The result of a $files query will contain objects with | ||
// metadata and a download URL you can use for serving files! | ||
const { $files: images } = data as { $files: Image[] }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You shouldn't need to write this if you use schema.
client/www/pages/docs/storage.md
Outdated
); | ||
const [isUploading, setIsUploading] = useState(false); | ||
|
||
if (isLoadingAvatar) return <div>Loading...</div>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: would return null
client/www/pages/docs/storage.md
Outdated
|
||
return <img src={imageUrl} />; | ||
function Wrapper() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: would call this App
or something
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor comments, but LGTM! Love the detail
); | ||
} | ||
|
||
function Wrapper() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: would write this as export default function Page
or something
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work!
client/www/pages/docs/storage.md
Outdated
|
||
{% callout type="warning" %} | ||
Now open `instant.perms.ts` and add the following permissions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: you mean instant.schema.ts
?
client/www/pages/docs/storage.md
Outdated
$files: i.entity({ | ||
"content-disposition": i.string().indexed(), | ||
"content-type": i.string().indexed(), | ||
"key-version": i.number(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope! Will remove!
client/www/pages/docs/storage.md
Outdated
Now open `instant.perms.ts` and add the following permissions | ||
|
||
```javascript {% showCopy=true %} | ||
import type { InstantRules } from "@instantdb/react"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
InstantRules are not needed here
client/www/pages/docs/storage.md
Outdated
@@ -5,287 +5,743 @@ title: Storage | |||
Instant Storage makes it simple to upload and serve files for your app. | |||
You can use Storage to store images, videos, documents, and any other file type. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use Storage to store images, videos, documents, and any other file type. | |
You can store images, videos, documents, and any other file type. |
client/www/pages/docs/storage.md
Outdated
|
||
Storage is still in **beta**, but you can request access [here](https://docs.google.com/forms/d/e/1FAIpQLSdzInffrNrsYaamtH_BUe917EOpcOq2k8RWcGM19XepJR6ivQ/viewform?usp=sf_link)! | ||
Let's use a fresh Next JS app to build a full example of how to upload and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use a fresh Next JS app to build a full example of how to upload and | |
Let's build a full example of how to upload and |
client/www/pages/docs/storage.md
Outdated
contentType: file.type, | ||
// See: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Disposition | ||
// Default: 'inline' | ||
contentDisposition: 'attachment; filename="moop.jpg"', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: did we mean the filename to be moop.jpg?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did want to show an example of setting a custom filename but perhaps we should be more descriptive!
client/www/pages/docs/storage.md
Outdated
``` | ||
|
||
Then, in your `ImageViewer` component, you can use the `cachedUrl` by default, and handle the expiration when necessary: | ||
Make sure to update this line with the app id in your `.env` file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If they have a .env file, perhaps it makes sense to simply use process.env.NEXT_PUBLIC_INSTANT_APP_ID
in the example?
client/www/pages/docs/storage.md
Outdated
const path = `${user.id}/orders/${orderId}.pdf`; | ||
await db.storage.uploadFile(path, file, { | ||
contentType: 'application/pdf', | ||
contentDisposition: 'attachment; filename="confirmation.pdf"', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: the filename
should be dynamic no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was actually intending not but we could do that!
client/www/pages/docs/storage.md
Outdated
}, | ||
}); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: perhaps these should be two separate code blocks?
client/www/pages/docs/storage.md
Outdated
|
||
The Admin SDK offers the same API for managing storage on the server, plus a few extra convenience methods for scripting. | ||
Here's a more detailed example showing how you may implement an avatar upload feature: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels a bit jarring -- it's a pretty big example, but feels like it's said nonchalantly. Perhaps we can give this a full subheader? We can show the example, and link out to the full repo? I am not sure but just felt a bit off scrolling here. At the least it's weird that with such a large codeblock there is no copy button
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure!
What it says on the tin!
Preview Link