Skip to content
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

[C3] fix: use a valid compatibility date for worker templates #3343

Merged
merged 3 commits into from
Jun 23, 2023

Conversation

petebacondarwin
Copy link
Contributor

Fixes #2385

What this PR solves / how to test:

Previously, we changed wrangler.toml to use the current date for the compatibility_date setting in wrangler.toml when generating workers. But this is almost always going to be new recent and results in a warning.

Now we look up the most recent compatibility date via npm on the workerd package and use that instead.

To test, build the create-cloudflare-package and then use it to build a workers project.
Check that the output displays:

├ Retrieving current workerd compatibility date 
│ compatibility date 2023-05-18

And that the generated wrangler.toml contains this date:

compatibility_date = "2023-05-18"

Associated docs issue(s)/PR(s):

  • N/A

Author has included the following, where applicable:

Reviewer is to perform the following, as applicable:

  • Checked for inclusion of relevant tests
  • Checked for inclusion of a relevant changeset
  • Checked for creation of associated docs updates
  • Manually pulled down the changes and spot-tested

@changeset-bot
Copy link

changeset-bot bot commented May 25, 2023

🦋 Changeset detected

Latest commit: 1073898

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

This PR includes changesets to release 1 package
Name Type
create-cloudflare 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

@github-actions
Copy link
Contributor

github-actions bot commented May 25, 2023

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/5356761586/npm-package-wrangler-3343

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

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

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/5356761586/npm-package-wrangler-3343 dev path/to/script.js
Additional artifacts:
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/5356761586/npm-package-cloudflare-pages-shared-3343

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

@codecov
Copy link

codecov bot commented May 25, 2023

Codecov Report

Merging #3343 (eab3088) into main (9ae3d93) will increase coverage by 0.02%.
The diff coverage is n/a.

❗ Current head eab3088 differs from pull request most recent head 1073898. Consider uploading reports for the commit 1073898 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3343      +/-   ##
==========================================
+ Coverage   75.07%   75.10%   +0.02%     
==========================================
  Files         183      183              
  Lines       11083    11083              
  Branches     2917     2917              
==========================================
+ Hits         8321     8324       +3     
+ Misses       2762     2759       -3     

see 2 files with indirect coverage changes

@petebacondarwin petebacondarwin force-pushed the fix-c3-compat-dates branch 2 times, most recently from abe40db to 8e20b9d Compare May 26, 2023 07:24
packages/create-cloudflare/src/workers.ts Outdated Show resolved Hide resolved
packages/create-cloudflare/src/workers.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@rozenmd rozenmd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, I'd probably wait for a review from someone more familiar with C3 though.

@petebacondarwin petebacondarwin force-pushed the fix-c3-compat-dates branch 7 times, most recently from d064d83 to 837c359 Compare May 27, 2023 10:38
@petebacondarwin
Copy link
Contributor Author

OK I have rebased and I think I have addressed all the comments.
My slight worry is that the type parameters are a bit messed up. But it works as is right now. We can probably be more sophisticated later?

Copy link
Contributor

@mrbbot mrbbot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! One minor comment, then I'll approve. 🙂

packages/create-cloudflare/src/helpers/command.ts Outdated Show resolved Hide resolved
Previously, we changed wrangler.toml to use the current date for the
compatibility_date setting in wrangler.toml when generating workers.
But this is almost always going to be new recent and results in a warning.

Now we look up the most recent compatibility date via npm on the workerd
package and use that instead.

Fixes cloudflare#2385
@petebacondarwin petebacondarwin merged commit cc9ced8 into cloudflare:main Jun 23, 2023
@petebacondarwin petebacondarwin deleted the fix-c3-compat-dates branch June 23, 2023 13:42
@github-actions github-actions bot mentioned this pull request Jun 22, 2023
lrapoport-cf pushed a commit that referenced this pull request Aug 11, 2023
* [C3] fix: use a valid compatibility date for worker templates

Previously, we changed wrangler.toml to use the current date for the
compatibility_date setting in wrangler.toml when generating workers.
But this is almost always going to be new recent and results in a warning.

Now we look up the most recent compatibility date via npm on the workerd
package and use that instead.

Fixes #2385

* Improve workerd date matching and support non-npm runners.

* fixup! [C3] fix: use a valid compatibility date for worker templates
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants