Docs

[jamesopstad]

resolves #87

This adds documentation to the readme. I've tried to keep the tutorial as succinct as possible but think it's a nice way to introduce the key concepts.

[pkg-pr-new]

Open in Stackblitz

npm i https://pkg.pr.new/flarelabs-net/vite-plugin-cloudflare/@flarelabs-net/vite-plugin-cloudflare@119

commit: 886873f

[jamesopstad]

Changes in this file are just to make the naming more consistent with the docs.

[IgorMinar]

Overall this looks really good and comprehensive to me. I left a few notes here and there with clarifications, but I think overall this make sense to me.

I'm approving this assuming that we resolve all the comments via edits an agreement to take no action.

Thank you @jamesopstad!

[IgorMinar]

This would also be a good place to mention that this plugin doesn't fully replace the need to have Wrangler installed as well and that wrangler is needed to read the config from wrangler.toml/json and deploy the final application.

[jamesopstad]

It might be a bit wordy to include this here before the Quick Start. Is it not clear that it's included as a dependency to install?

[petebacondarwin]

LGTM once Igor and Dario's comments have been addressed

[petebacondarwin]

Does the Vite React creation tool generate the tsconfig.app.json and tsconfig.node.json?
If so, how come it doesn't already include the TypeScript dependency?

[jamesopstad]

It does generate those files and includes the TypeScript dependency. Not quite sure what you mean?

[dario-piotrowicz]

A minor comment that just occurred to me, given the recent push for wrangler.json (cloudflare/workers-sdk#7676, etc...) should we maybe switch our wrangler files into jsons? and also refer to the json format in the docs?

I think that'd make sense, it also doesn't need to be addressed in this PR (since it would probably be good to also update the config files in the various playground apps)

One concern I have docs wise is that now we get a bit of a stronger distinction between the userland/input wrangler.toml and the output wrangler.json, if we had wrangler.json both as the input and output that could potentially confuse some people if not conveyed very clearly 🤔

[ToriLindsay]

Just left a bunch of grammar/clarity suggestions. Feel free to ignore any that don't make sense for the context.
Note: We always use the oxford comma.
Also, I noticed that it was jumping between the first, second, and third person a lot, so I removed the "we", "us", "let's", etc. to keep a little more consistent.

[dario-piotrowicz]

Should we have a more detailed description? (briefly mentioning what the plugin does/what it can be used for etc...)

For the parameter, why not a standard @param tag? 😮

[jamesopstad]

To be honest, I just followed Vite's example here - https://github.com/vitejs/vite/blob/4f5845a3182fc950eb9cd76d7161698383113b18/packages/vite/src/node/config.ts#L137-L141.

Feel free to make a suggestion.

[dario-piotrowicz]

What about something along these lines?

This is how something like that would get rendered in vscode:

[jamesopstad]

Thanks. Let's add the JSDoc once the readme is in workers-sdk so it points to the right place.

[petebacondarwin]

Added some thoughts about the environments section.

[petebacondarwin]

Suggested change

	A Worker config file may contain configuration for multiple [environments](https://developers.cloudflare.com/workers/wrangler/environments/).
	A Worker config file may contain configuration for multiple [Wrangler environments](https://developers.cloudflare.com/workers/wrangler/environments/).

[petebacondarwin]

or possibly

Suggested change

	A Worker config file may contain configuration for multiple [environments](https://developers.cloudflare.com/workers/wrangler/environments/).
	A Worker config file may contain configuration for multiple [Cloudflare environments](https://developers.cloudflare.com/workers/wrangler/environments/).

[petebacondarwin]

Oh I see below that you use the term "Worker environment" - so perhaps that would be best here.

[jamesopstad]

I've called the section 'Worker environments`. I don't think there's any option here that isn't a little confusing because the word 'environments' is used for so many things. I do think that calling them 'Wrangler environments' would potentially be even more confusing though.

[jamesopstad]

I've updated it to 'Worker environments'. Let me know if you would rather I use the term 'Cloudflare environments' throughout the readme though. I'm not sure which is better.