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

Refactor index.tsx (and config.ts) to pass strict-null checks #249

Merged

Conversation

petebacondarwin
Copy link
Contributor

This actually slightly changes the processing of the environment configuration.
Previously there were corner cases where the configuration might just crash.
These are now handled more cleanly with more appropriate warnings.

The yargs commands have been refactored slightly to be more consistent.
Now each command that can be either in local or remote mode is structured the same way.
The config is extracted from the args, then there is an if-then-else statement that handles the local and remote cases.
This ensures that the account_id has the correct typings in remote mode as needed.

@changeset-bot
Copy link

changeset-bot bot commented Jan 15, 2022

🦋 Changeset detected

Latest commit: d218f2e

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

Copy link
Contributor

@threepointone threepointone left a comment

Choose a reason for hiding this comment

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

This is looking good, dropped a couple of notes. If you could bring back the snip comments I'd be happy to approve and land this and work on it further. thank you!

packages/wrangler/src/config.ts Outdated Show resolved Hide resolved
packages/wrangler/src/index.tsx Show resolved Hide resolved
Copy link
Contributor

@threepointone threepointone left a comment

Choose a reason for hiding this comment

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

approving with one last note. thanks!

packages/wrangler/src/index.tsx Show resolved Hide resolved
This actually slightly changes the processing of the environment configuration.
Previously there were corner cases where the configuration might just crash.
These are now handled more cleanly with more appropriate warnings.

The yargs commands have been refactored slightly to be more consistent.
Now each command that can be either in local or remote mode is structured the same way.
The `config` is extracted from the args, then there is an `if-then-else` statement that handles the local and remote cases.
This ensures that the `account_id` has the correct typings in remote mode as needed.
@petebacondarwin petebacondarwin merged commit 9769bc3 into cloudflare:main Jan 20, 2022
@github-actions github-actions bot mentioned this pull request Jan 20, 2022
@petebacondarwin petebacondarwin deleted the strict-null-checks-index branch January 20, 2022 13:05
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