Skip to content

Conversation

@rviscomi
Copy link
Contributor

@rviscomi rviscomi commented Mar 11, 2025

Prerequisites checklist

What is the purpose of this pull request?

Adds the ability to configure the require-baseline rule to target a Baseline year as an alternative to newly or widely available status

What changes did you make? (Give an overview)

  • added the integer type as an alternative to the available configuration property enum
  • created a BaselineAvailability class to determine whether a feature is supported based on the config
  • added tests that use the year config
  • generated feature years in addition to their status ID
  • regenerated baseline-data.js
  • updated docs
  • ran the formatter, which generated an unrelated change to README.md

Related Issues

fixes #78

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

For the rule schema, would you prefer to configure the baseline year as a separate optional property, rather than shoehorning it in with available?

@nzakas nzakas marked this pull request as draft March 11, 2025 15:32
@nzakas
Copy link
Member

nzakas commented Mar 11, 2025

Thanks for putting this together. I'm not sure how useful this feature is, so let's discuss back on #78 before iterating on the code.

@nzakas nzakas marked this pull request as ready for review March 12, 2025 14:24
Copy link
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.

I just left a few cleanup notes. Overall this looks good.

Do we want to wait to merge #82 before merging this?

@fasttime fasttime added this to Triage Mar 12, 2025
@fasttime fasttime moved this to Needs Triage in Triage Mar 12, 2025
@rviscomi
Copy link
Contributor Author

Thanks @nzakas this is ready for another look

Do we want to wait to merge #82 before merging this?

No preference. Merge conflicts either way 😁

@nzakas nzakas moved this from Needs Triage to Implementing in Triage Mar 13, 2025
Copy link
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.

Almost there, just left a few comments around optimization.

// baseline year
type: "integer",
minimum: 2000,
maximum: new Date().getFullYear() + 1,
Copy link
Member

Choose a reason for hiding this comment

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

Double-checking: is this intentional? The max now would be 2026?

Or did you mean this?

Suggested change
maximum: new Date().getFullYear() + 1,
maximum: new Date().getFullYear() - 1,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, yes this was intentional (see #81 (comment)) but I just realized the maximum is inclusive. My reasoning was that the current year is technically a valid Baseline year, ie https://web.dev/baseline/2025.

Fixed this to use the current year, but let me know if you'd still prefer YYYY - 1.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good to me. 👍

Copy link
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. Thanks for all of your work on this.

@nzakas nzakas merged commit 4c70882 into eslint:main Mar 14, 2025
22 checks passed
@github-project-automation github-project-automation bot moved this from Implementing to Complete in Triage Mar 14, 2025
@rviscomi rviscomi deleted the baseline-year branch March 14, 2025 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Complete

Development

Successfully merging this pull request may close these issues.

Change Request: add support for Baseline years to require-baseline

2 participants