workers-sdk icon indicating copy to clipboard operation
workers-sdk copied to clipboard

Remove warnings, outdated templates, proxy commands to C3

Open irvinebroque opened this issue 1 year ago • 2 comments

What

  1. Removes the outdated directory of templates in /templates. None of these are actually up-to-date. As-needed some of this content can be added to npm create cloudflare templates that live here.
  2. wrangler generate and wrangler init both just kick you over to npm create cloudflare automatically now. wrangler generate template <repository_url> behavior still works, just now via create cloudflare.
  3. Removes a lot of old code

irvinebroque avatar Aug 03 '24 03:08 irvinebroque

🦋 Changeset detected

Latest commit: 337043f1bd89e99fb1403a8ec89255c735d91370

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
wrangler Minor
@cloudflare/vitest-pool-workers Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

changeset-bot[bot] avatar Aug 03 '24 03:08 changeset-bot[bot]

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/10852307068/npm-package-wrangler-6415

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/6415/npm-package-wrangler-6415

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/10852307068/npm-package-wrangler-6415 dev path/to/script.js
Additional artifacts:
npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/10852307068/npm-package-create-cloudflare-6415 --no-auto-update
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/10852307068/npm-package-cloudflare-kv-asset-handler-6415
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/10852307068/npm-package-miniflare-6415
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/10852307068/npm-package-cloudflare-pages-shared-6415
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/10852307068/npm-package-cloudflare-vitest-pool-workers-6415
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/10852307068/npm-package-cloudflare-workers-editor-shared-6415
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/10852307068/npm-package-cloudflare-workers-shared-6415

Note that these links will no longer work once the GitHub Actions artifact expires.


[email protected] includes the following runtime dependencies:

Package Constraint Resolved
miniflare workspace:* 3.20240909.0
workerd 1.20240909.0 1.20240909.0
workerd --version 1.20240909.0 2024-09-09

Please ensure constraints are pinned, and miniflare/workerd minor versions match.

github-actions[bot] avatar Aug 03 '24 03:08 github-actions[bot]

All of my examples at https://github.com/cloudflare/workers-sdk/issues/6459 seem to be using the old code-path for generate, so I don't think this is (generally) semver breaking anymore with old generate URLs, thanks all. I appreciate the attention given here to ensure this didn't introduce major breaking changes to existing command behaviour in v3.

Cherry avatar Sep 12 '24 22:09 Cherry

@IgorMinar this change is not backwards compatible if users are generating code with named templates. An old version of Wrangler, for example, will error if trying something like wrangler generate my-worker some-template as the template location no longer exists.

For new versions of Wrangler, this delegates directly to C3. There is no mapping between templates because C3 templates do not correspond with the templates in the templates folder - there is almost no overlap there. It therefore delegates to C3 with the defaults (without deploy).

andyjessop avatar Sep 13 '24 08:09 andyjessop

Thanks @andyjessop for the explanation, and @Cherry for additional info and pointer to https://github.com/cloudflare/workers-sdk/issues/6459 - which I believe we can close as soon as this PR is merged, right?

I've reviewed this case and here is my decision in a few bullet points:

  • this is a breaking change, and at odds with semver, however...
  • we don't really understand the real world impact of this breaking change because we don't have analytics that would tell us how often wrangler generate is invoked with a template name option
  • my estimate based on some research that the impact is likely close to 0 (I couldn't find docs or other references where we would instruct users to use wrangler generate with a template name anywhere except for a few 3rd party blog posts)
  • @irvinebroque has initiated this breaking change and approved it from the product perspective, so we have no blockers there
  • it is however wrong and unexpected that if a user calls wrangler generate my-worker some-template we give them c3 flow, and ignore the template name — this is a bug that we should address it, however not today
  • it's Friday, we should get this change in because overall it's valuable and Monday is feature freeze for bday week
    • we already believe the impact of the breaking change is minimal, we can live with the bug described in the previous point for a few days

Next steps:

  • [x] merge this now as is and release it
  • [ ] file a bug describing the issue above
  • [ ] fix the bug next week by throwing a descriptive error when the user invokes wrangler generate with the template name option, and tell them that this feature is not available and instead they can just use generic c3 flow. Or even prompt them if they'd like us to invoke for them instead.
  • [ ] escalate the analytics issue as a blocker for proper decision making which is causing us to spin in circles of endless discussions about what might or might not be the usage of various features. this is a major waste and productivity drag on all of us.

that's all.. does this all matter? in terms of user impact likely not, but if we claim that we want to change our culture and improve DX across the board, then this is the level of attention we need to pay to these changes, and we should implement this level of polish (the bug fix described above) in all the scenarios where we are unsure about the real world impact. Only in cases where we believe that (and have data to prove it) the impact is 0, we can skip the polish.

IgorMinar avatar Sep 13 '24 19:09 IgorMinar

and I see that @penalosa already merged this. thank you!

IgorMinar avatar Sep 13 '24 19:09 IgorMinar