Skip to content

Add more helpful error messages when validating compatibility date#7002

Merged
CarmenPopoviciu merged 2 commits intomainfrom
more-compat-date-validation
Dec 9, 2024
Merged

Add more helpful error messages when validating compatibility date#7002
CarmenPopoviciu merged 2 commits intomainfrom
more-compat-date-validation

Conversation

@GregBrimble
Copy link
Contributor

@GregBrimble GregBrimble commented Oct 17, 2024

What this PR solves / how to test

Fixes WC-2854.

Screenshot 2024-12-09 at 17 35 09

Author has addressed the following

  • Tests
    • TODO (before merge)
    • Tests included
    • Tests not necessary because:
  • E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
    • I don't know
    • Required
    • Not required because: no e2e coverage
  • Changeset (Changeset guidelines)
    • TODO (before merge)
    • Changeset included
    • Changeset not necessary because:
  • Public documentation
    • TODO (before merge)
    • Cloudflare docs PR(s):
    • Documentation not necessary because: just adding better errors :)

@GregBrimble GregBrimble requested a review from a team as a code owner October 17, 2024 04:49
@changeset-bot
Copy link

changeset-bot bot commented Oct 17, 2024

🦋 Changeset detected

Latest commit: d25e501

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

This PR includes changesets to release 2 packages
Name Type
wrangler Patch
@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

@github-actions
Copy link
Contributor

github-actions bot commented Oct 17, 2024

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/12240002379/npm-package-wrangler-7002

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

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

Or you can use npx with this latest build directly:

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

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


wrangler@3.93.0 includes the following runtime dependencies:

Package Constraint Resolved
miniflare workspace:* 3.20241205.0
workerd 1.20241205.0 1.20241205.0
workerd --version 1.20241205.0 2024-12-05

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

@GregBrimble GregBrimble force-pushed the more-compat-date-validation branch from e5e2db7 to 6eea915 Compare October 17, 2024 05:01
@GregBrimble GregBrimble force-pushed the more-compat-date-validation branch from cbb4b61 to 6eea915 Compare October 17, 2024 19:19
@CarmenPopoviciu CarmenPopoviciu self-assigned this Nov 7, 2024
@CarmenPopoviciu
Copy link
Contributor

picking this up to shepherd across the finish line 💟

@CarmenPopoviciu CarmenPopoviciu force-pushed the more-compat-date-validation branch from 6eea915 to 4666749 Compare December 5, 2024 10:53
isValid = false;

diagnostics.errors.push(
`"${field}" field should use hyphens (-) rather than en-dashes (—) or em-dashes (–).`
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a better way to demonstrate this, given that all three look identical in monospace fonts? Maybe there isn't, but I thought I would challenge it just because it reads very strangely in the terminal.

Copy link
Contributor

Choose a reason for hiding this comment

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

not that I can tell I'm afraid. I totes get what you mean, and I did look into it, but it doesn't seem to be possible for us to control how the user's terminal will display these things.

Copy link
Contributor

Choose a reason for hiding this comment

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

NP! Thanks for looking into it.

@CarmenPopoviciu CarmenPopoviciu force-pushed the more-compat-date-validation branch from 4666749 to d25e501 Compare December 9, 2024 16:32
@CarmenPopoviciu CarmenPopoviciu merged commit d2447c6 into main Dec 9, 2024
@CarmenPopoviciu CarmenPopoviciu deleted the more-compat-date-validation branch December 9, 2024 16:58
@workers-devprod workers-devprod mentioned this pull request Dec 9, 2024
penalosa pushed a commit that referenced this pull request Jan 10, 2025
…ibility date (#7002)

* Add more helpful error messages when validating compatibility date

* PR feedback

---------

Co-authored-by: Carmen Popoviciu <cpopoviciu@cloudflare.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants