Revised the extension sample README.md

[0xakihiko]

I found the example code great, but as there are a few very similarly named files for the extension sample with two different ways of using the burrito-shop, it wasn't clear without a deep look into exactly what was happening or why specific things were defined as they were.

As such, I put my thoughts into a README.md so the next person doesn't have to build the same mental model from scratch.

Ideally I'd have split this into two different samples, with one being the environment binding and another being the imported module, but I choose to use this as a stepping stone since it's my first PR.

[github-actions]

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

[0xakihiko]

I have read the CLA Document and I hereby sign the CLA

[ohodson]

Thanks for improving the documentation here.

Just a couple of review comments, but otherwise lgtm.

We do most of workerd development in vscode; if you're using that there is a good markdownlint extension that flags common markdown issues.

[0xakihiko]

Thanks for improving the documentation here.

Just a couple of review comments, but otherwise lgtm.

We do most of workerd development in vscode; if you're using that there is a good markdownlint extension that flags common markdown issues.

Appreciate you taking the time to review it.

After your comments I had a second look and found some other areas for improvement(such as defining the codeblocks as capnp to be in line with the rest of the repo), I'll submit the changes soon as well as with your suggested changes.

Also cheers for the suggested extension, I'll give it a try.

[ohodson]

Suggested change

	This method demonstrates a publicly importable module, with the initialization being handled by the worker in [worker.js](worker.js)
	This method demonstrates a publicly importable module, with the initialization being handled by the worker in [worker.js](worker.js)

[ohodson]

LGTM: it is a very useful update to the docs. Thanks!

[mikea]

@0xakihiko thanks for updating the readme. I did consider separating samples out, but custom binding and import features are so interlinked, it was hard to imagine independent examples.

I'll also be frank with you that I struggled to come up with a more useful/functional example that wouldn't require writing and maintaining lots of code. Ideas/contributions here are more than welcome.

[mikea]

@ohodson thanks for review

[0xakihiko]

@0xakihiko thanks for updating the readme. I did consider separating samples out, but custom binding and import features are so interlinked, it was hard to imagine independent examples.

I'll also be frank with you that I struggled to come up with a more useful/functional example that wouldn't require writing and maintaining lots of code. Ideas/contributions here are more than welcome.

Well, I did find your example exactly what I needed for my specific use-case, so I'd like to add that you demonstrated a valid use-case in a way that I instantly knew it was what I needed when I saw it.

The only issue I had was that you're demonstrating two different methods - but it didn't actually occur to me at first since I tend to actually go to the low-level(the backing implementations) before I read the usage of it (worker.js). Take this as a bad habit from my auditing days. This was a mistake on my part- as it didn't really occur to me until I had built my own test-cases on my project that you were demostrating two different distinct ways to use the burrito shop until I saw the worker.js and then it made sense.

In my own test-cases, I have built a few extensions in typescript, then I build the esmodule and output type definitions.
I also utilize these bindings to avoid including a lot of boilerplate code to try keep built esmodules only to business logic, and not say...'node:http'.

The type definitions are provided to the user similar to @cloudflare/workers-types.

The demonstration is great and your code is succinct, I think you'd be hard pressed to find a way to demonstrate it with less code.

If you'd like, I think I could add another sample or two that follows something like the below.

Imports

For me, I think this is a great way to provide imports to avoid having user modules include common functionality like node:http etc. In my case, I have workers that are built into esmodules, with certain imports being marked external so they're not included - they're then included as a binding to the worker. This could be seen as an added benefit of being able to roll out critical security fixes without having to rebuild modules. It could also be another way to provide a module that is written in rust etc that the user doesn't necessarily have to bundle, or can't. It's amazingly flexible.

A simple example though could be allowing a user work modules to stay business logic only, and reduces code size. So while I would want to make it typescript etc and utilize externals etc, I don't believe my way of doing it (esbuild, custom plugins, etc) would be concise or a 'best practice'.

I'd have to think on this one more, since I do think the way you've done it is probably hard to beat as I believe my example is not an import example for hiding implementation through extensions - but more of a way to avoid bundling library code.

Env Binding

For me, I'd have probably gone with a simple in-memory KV, but thinking on that more after reading what you've said I believe this is far too real-world use case, and as there are many other ways to do this, and other samples, it could be a "so which way should we provide a KV". I do think the burrito shop was a good use-case.

A great benefit to this method over the imports is that you're able to provide environment variables to the module, and then have that be used to create the instance by workerd. This means users are not responsible for creating the module or it's initialization. You had already demonstrated it like this, and it's a great way to do it.

So ideally it would utilize this in some way. I'm not sure of the environment variable isolation for this, and as such I'd probably be avoiding using examples that utilize these env vars in a way, or the promise of the implementation of it to forever stay isolated - so I'd like to avoid using it in a way that'd be seen as a recommendation to use it for secrets that we don't want a service to have, only the binding burritoshop(e.g: an api-key for taking burritos from an outside HTTP service instead of in-memory from inventory).

I'd continue with the burrito shop example as such, but possibly do it as the following:

The bindings in this case, instead of a recipe it could have burritoType/inventory. (I would have preferred cost instead of inventory, but this would mean I'd maybe have to provide some form of balance management)

From there, we provide get(burritoType) and count(burritoType). At that point, it's just simply env.burritoShop.get(burritoType).

tl;dr Basically, I think the example is great and I don't really think I could do much more than just cutting them up. The only changes I could make would probably be whole new samples that may 'inspire' ways to use workerd.