-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
docs(guides/data-loading): add Cloudflare Pages reference to Cloudflare KV section #3718
Conversation
Instructions for Workers were confusing to me when I was figuring out how to set up KV in CF Pages. I hope this could help.
|
Hi @vorcigernix, Welcome, and thank you for contributing to Remix! Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once. You may review the CLA and sign it by adding your name to contributors.yml. Once the CLA is signed, the If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at [email protected]. Thanks! - The Remix team |
Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳 |
docs/guides/data-loading.md
Outdated
@@ -229,8 +229,40 @@ export default function Product() { | |||
``` | |||
|
|||
## Cloudflare KV | |||
If you picked Cloudflare Pages as your environment, [Cloudflare Key Value][cloudflare-kv] storage allows you to persist data at the edge as if it were a static resource. To start with local development, you need to add a ```--kv``` parameter with a name of your namespace to the package.json task, so it would look like this: |
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 you picked Cloudflare Pages as your environment, [Cloudflare Key Value][cloudflare-kv] storage allows you to persist data at the edge as if it were a static resource. To start with local development, you need to add a ```--kv``` parameter with a name of your namespace to the package.json task, so it would look like this: | |
If you picked Cloudflare Pages as your environment, [Cloudflare Key Value][cloudflare-kv] storage allows you to persist data at the edge as if it were a static resource. To start with local development, you need to add a `--kv` parameter with a name of your namespace to the package.json task, so it would look like this: |
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.
Sorry, I'm confused. It looks like you've duplicated the example 🤔
I almost did. Wanted to keep it the same, not sure how to rephrase to make
sure the difference is clear. I wanted to add a part where I struggled
without influencing current documentation on workers (I don't use them, at
least not directly). I may try to condensate it even more by pointing out
only differences in example, but I was afraid that this could be confusing
as well.
…On Wed, 13 Jul 2022 at 18:00, Kent C. Dodds ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Sorry, I'm confused. It looks like you've duplicated the example 🤔
------------------------------
In docs/guides/data-loading.md
<#3718 (comment)>:
>
-If you picked Cloudflare Workers as your environment, [Cloudflare Key Value][cloudflare-kv] storage allows you to persist data at the edge as if it were a static resource. You'll need to [do some configuration][cloudflare-kv-setup] but then you can access the data from your loaders:
+```"dev:wrangler": "cross-env NODE_ENV=development wrangler pages dev ./public --kv PRODUCTS_KV"```
⬇️ Suggested change
-```"dev:wrangler": "cross-env NODE_ENV=development wrangler pages dev ./public --kv PRODUCTS_KV"```
+```
+"dev:wrangler": "cross-env NODE_ENV=development wrangler pages dev ./public --kv PRODUCTS_KV"
+```
------------------------------
In docs/guides/data-loading.md
<#3718 (comment)>:
>
-If you picked Cloudflare Workers as your environment, [Cloudflare Key Value][cloudflare-kv] storage allows you to persist data at the edge as if it were a static resource. You'll need to [do some configuration][cloudflare-kv-setup] but then you can access the data from your loaders:
+```"dev:wrangler": "cross-env NODE_ENV=development wrangler pages dev ./public --kv PRODUCTS_KV"```
+
+This enable you to use the PRODUCTS_KV in a loader context:
⬇️ Suggested change
-This enable you to use the PRODUCTS_KV in a loader context:
+This enable you to use the `PRODUCTS_KV` in a loader context:
------------------------------
In docs/guides/data-loading.md
<#3718 (comment)>:
> + const product = useLoaderData();
+ return (
+ <div>
+ <p>{} Products</p>
+ {/* ... */}
+ </div>
+ );
⬇️ Suggested change
- const product = useLoaderData();
- return (
- <div>
- <p>{} Products</p>
- {/* ... */}
- </div>
- );
+ const product = useLoaderData();
+ return (
+ <div>
+ <p>Product</p>
+ {product.name}
+ </div>
+ );
—
Reply to this email directly, view it on GitHub
<#3718 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABLZODNLBKJ77TMBKXDH6TVT3R3FANCNFSM53OUN3ZA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
What is the difference in the code examples? |
Sorry, was replying from mobile. Suggested changes make total sense, now I
understand your reply. Will change later today.
…On Wed, Jul 13, 2022, 9:59 PM Kent C. Dodds ***@***.***> wrote:
What is the difference in the code examples?
—
Reply to this email directly, view it on GitHub
<#3718 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABLZOFVCJTNKVV7QCI2IFDVT4NZDANCNFSM53OUN3ZA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Co-authored-by: Kent C. Dodds <[email protected]>
Co-authored-by: Kent C. Dodds <[email protected]>
Co-authored-by: Kent C. Dodds <[email protected]>
Accepted changes and made the description more concise.
docs/guides/data-loading.md
Outdated
```tsx filename=app/routes/products/$productId.tsx | ||
import type { LoaderFunction } from "@remix-run/node"; // or "@remix-run/cloudflare" | ||
import { json } from "@remix-run/node"; // or "@remix-run/cloudflare" | ||
This enable you to use the `PRODUCTS_KV` in a loader context: |
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.
Can you show how you get PRODUCTS_KV
into the loader context?
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.
The context is initiated on line 245 and it is used on line 248. Pages for some reason require this, while Workers does not.
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.
It's probably better to just generalize this into something about "getLoadContext" is the place where you can pass CF env things through to your action / loader context so it can be applicable to Workers / Pages.
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.
Good point. Do we do the same thing with context
for workers? You can get things off of context
in that environment as well?
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.
Actually, fixing this behavior to have the same code for both platforms would be great. I consider the Pages code easier to understand though, context explains where the namespace comes from.
Co-authored-by: Kent C. Dodds <[email protected]>
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.
Thanks!
…re KV section (#3718) Co-authored-by: Kent C. Dodds <[email protected]>
…re KV section (remix-run#3718) Co-authored-by: Kent C. Dodds <[email protected]>
Instructions for Workers were confusing to me when I was figuring out how to set up KV in CF Pages. I hope this could help.