Skip to content

fix: non-optional properties in generic interfaces#132

Merged
aladdin-add merged 1 commit into
mainfrom
fix-generic-interfaces
Nov 7, 2024
Merged

fix: non-optional properties in generic interfaces#132
aladdin-add merged 1 commit into
mainfrom
fix-generic-interfaces

Conversation

@fasttime
Copy link
Copy Markdown
Member

@fasttime fasttime commented Nov 6, 2024

Prerequisites checklist

What is the purpose of this pull request?

Fix a bug in the types.

What changes did you make? (Give an overview)

This PR fixes the type integration tests in ESLint which would currently fail with @eslint/core@0.8.0 (eslint/eslint#19098).

The fix consists in marking the properties of generic interfaces as non-optional, meaning that they can't be undefined. These properties are only used to refer to a type. For example there is no actual object with a property MessageIds, but RuleDefinitionTypeOptions["MessageIds"] is used as an alias for string. If a union with undefined is desired, it can always be indicated explicitly as RuleDefinitionTypeOptions["MessageIds"] | undefined.

Related Issues

refs eslint/eslint#19098

Is there anything you'd like reviewers to focus on?

@eslint-github-bot eslint-github-bot Bot added the bug Something isn't working label Nov 6, 2024
@fasttime fasttime marked this pull request as ready for review November 6, 2024 06:16
Copy link
Copy Markdown
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM. Would like another review before merging.

Copy link
Copy Markdown
Member

@aladdin-add aladdin-add left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@aladdin-add aladdin-add merged commit d0b2e70 into main Nov 7, 2024
@aladdin-add aladdin-add deleted the fix-generic-interfaces branch November 7, 2024 10:12
@github-actions github-actions Bot mentioned this pull request Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Status: Complete

Development

Successfully merging this pull request may close these issues.

3 participants