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

fix: set default field to any and fix validator function return type #6690

Merged
merged 2 commits into from
Dec 9, 2022

Conversation

btw17
Copy link
Member

@btw17 btw17 commented Dec 9, 2022

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide
  • I ran npm run format and npm run lint

The details

Resolves

Although the default Field type T should eventually be unknown, set it to any so it does not break users with subclasses extending from Field in the meantime.

Proposed Changes

  • Update core/field.ts - FieldValidator to provide the correct return type.
  • Update core/field.ts - T = unknown to T = any

Behavior Before Change

TypeScript users with classes extending from Field would have to fill in a type for Field or their code will break.

Behavior After Change

TypeScript users can now extend from Field without providing the value type.

Reason for Changes

This is for introducing the Field type information in multiple stages.

Test Coverage

N/A

Documentation

N/A

Additional Information

Nope!

@btw17 btw17 requested a review from a team as a code owner December 9, 2022 22:02
@btw17 btw17 requested a review from maribethb December 9, 2022 22:02
@github-actions github-actions bot added the PR: chore General chores (dependencies, typos, etc) label Dec 9, 2022
Copy link
Contributor

@maribethb maribethb left a comment

Choose a reason for hiding this comment

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

LGTM, but can you call this a fix instead of a chore? Chores will now be excluded from the release notes and should only be for internal-facing things developers have no reason to care about.

Thanks!

@btw17 btw17 changed the title chore: set default field to and fix validator function return type fix: set default field to and fix validator function return type Dec 9, 2022
@github-actions github-actions bot added PR: fix Fixes a bug and removed PR: chore General chores (dependencies, typos, etc) labels Dec 9, 2022
@maribethb maribethb changed the title fix: set default field to and fix validator function return type fix: set default field to any and fix validator function return type Dec 9, 2022
@github-actions github-actions bot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels Dec 9, 2022
@maribethb maribethb merged commit 9ee1559 into google:develop Dec 9, 2022
@btw17 btw17 deleted the chore/set-default-field-to-any branch December 9, 2022 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: fix Fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants