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

Add --compatibility-date, --compatibility-flags, --latest cli arguments to dev and publish #215

Merged
merged 3 commits into from
Jan 11, 2022

Conversation

threepointone
Copy link
Contributor

@threepointone threepointone commented Jan 7, 2022

Closes #193.

This PR adds cli fields for 2 existing configuration fields: --compatibility-date, --compatibility-flags matching compatibility_date and compatibility_flags respectively. It also adds a cli arg --latest which is shorthand for --compatibility_date <today>. It follows the rules from the linked issue -

  • A cli arg for adding a compatibility data, e.g --compatibility_date 2022-01-05
  • A shorthand --latest that sets compatibility_date to today's date. Usage of this flag logs a warning.
  • latest is NOT a config field in wrangler.toml.
  • In dev, when a compatibility date is not available in either wrangler.toml or as a cli arg, then we default to --latest.
  • In publish we error if a compatibility date is not available in either wrangler.toml or as a cli arg. Usage of --latest logs a warning.
  • We also accept compatibility flags via the cli, e.g: --compatibility-flags formdata_parser_supports_files

I haven't added validation for the actual values of these args, I'll get to that next week when I work on the config validation work. The wording of the messages/errors are also up for discussion. I'm happy to keep this PR alive until we get it right.

@changeset-bot
Copy link

changeset-bot bot commented Jan 7, 2022

🦋 Changeset detected

Latest commit: 3b89849

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

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

…rguments to `dev` and `publish`

Closes #193.

This PR adds cli fields for 2 existing configuration fields: `--compatibility-date`, `--compatibility-flags` matching `compatibility_date` and `compatibility_flags` respectively. It also adds a cli arg `--latest` which is shorthand for `--compatibility_date <today>`. It follows the rules from the linked issue -

- A cli arg for adding a compatibility data, e.g  `--compatibility_date 2022-01-05`
- A shorthand `--latest` that sets `compatibility_date` to today's date. Usage of this flag logs a warning.
- `latest` is NOT a config field in `wrangler.toml`.
- In `dev`, when a compatibility date is not available in either `wrangler.toml` or as a cli arg, then we default to `--latest`, and log a warning.
- In `publish` we error if a compatibility date is not available in either `wrangler.toml` or as a cli arg. Usage of `--latest` logs a warning.
- We also accept compatibility flags via the cli, e.g: `--compatibility-flags formdata_parser_supports_files`

I haven't added validation for the actual values of these args, I'll get to that next week when I work on the config validation work. The wording of the messages/errors are also up for discussion. I'm happy to keep this PR alive until we get it right.
@petebacondarwin
Copy link
Contributor

In dev, when a compatibility date is not available in either wrangler.toml or as a cli arg, then we default to --latest, and log a warning.

I think logging a warning for dev is not really necessary. The vast majority of the time users will just want to develop against latest, unless they specifically want to lock it down to a date - in which case they would be best applying it in a configuration. There will also be an error if they try to publish.

type: "array",
alias: "compatibility-flag",
})
.option("latest", {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we error if compatibility-date and latest are both used?
Should we log a warning if there is a compatibility-date set in config and is being overridden by --latest?

Copy link
Contributor Author

@threepointone threepointone Jan 11, 2022

Choose a reason for hiding this comment

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

I... hrmm. I'd push back against the error; the use case is the commands been generated by some other tooling, or literally just inside an npm script, and you're overriding it manually to try the latest runtime version. Would be annoying if an error blocked you then. We have the warning anyway, and it's an annoying one that you can't miss.

"⚠️ Using the latest version of the Workers runtime. To silence this warning, please choose a specific version of the runtime with --compatibility-date, or add a compatibility_date to your wrangler.toml.\n"
);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with @petebacondarwin that the warning during dev is unnecessary. Removing it.

type: "array",
alias: "compatibility-flag",
})
.option("latest", {
Copy link
Contributor Author

@threepointone threepointone Jan 11, 2022

Choose a reason for hiding this comment

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

I... hrmm. I'd push back against the error; the use case is the commands been generated by some other tooling, or literally just inside an npm script, and you're overriding it manually to try the latest runtime version. Would be annoying if an error blocked you then. We have the warning anyway, and it's an annoying one that you can't miss.

Copy link
Contributor

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

EASY LGTM

@threepointone threepointone merged commit 41d4c3e into main Jan 11, 2022
@threepointone threepointone deleted the compat-date branch January 11, 2022 14:52
@github-actions github-actions bot mentioned this pull request Jan 11, 2022
threepointone added a commit that referenced this pull request Jan 18, 2022
We introduced some bugs in recent PRs:

- In #196, we broke being able to pass an entrypoint directly to the cli. In this PR, I just reverted that fix. I'll reopen #78 and we'll tackle it again later. (cc @jgentes)
- In #215, we broke being able to publish a script by just passing `--latest` or `--compatibility-data` in the cli. This PR fixes that by reading the correct argument when choosing whether to publish.
- In #247, we broke how we made requests by passing headers to requests. This PR reverts the changes made in `cfetch/internal.ts`. (cc @petebacondarwin)
- In #244, we broke `dev` and it would immediately crash. This PR fixes the reference in `dev.tsx` that was breaking. (cc @petebacondarwin)
threepointone added a commit that referenced this pull request Jan 18, 2022
We introduced some bugs in recent PRs:

- In #196, we broke being able to pass an entrypoint directly to the cli. In this PR, I just reverted that fix. I'll reopen #78 and we'll tackle it again later. (cc @jgentes)
- In #215, we broke being able to publish a script by just passing `--latest` or `--compatibility-data` in the cli. This PR fixes that by reading the correct argument when choosing whether to publish.
- In #247, we broke how we made requests by passing headers to requests. This PR reverts the changes made in `cfetch/internal.ts`. (cc @petebacondarwin)
- In #244, we broke `dev` and it would immediately crash. This PR fixes the reference in `dev.tsx` that was breaking. (cc @petebacondarwin)
threepointone added a commit that referenced this pull request Jan 18, 2022
We introduced some bugs in recent PRs:

- In #196, we broke being able to pass an entrypoint directly to the cli. In this PR, I just reverted that fix. I'll reopen #78 and we'll tackle it again later. (cc @jgentes)
- In #215, we broke being able to publish a script by just passing `--latest` or `--compatibility-data` in the cli. This PR fixes that by reading the correct argument when choosing whether to publish.
- In #247, we broke how we made requests by passing headers to requests. This PR reverts the changes made in `cfetch/internal.ts`. (cc @petebacondarwin)
- In #244, we broke `dev` and it would immediately crash. This PR fixes the reference in `dev.tsx` that was breaking. (cc @petebacondarwin)
@github-actions github-actions bot mentioned this pull request Jan 18, 2022
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.

nuance of compatibility_date behaviour.
2 participants