Skip to content

feat: add support for baseline years #81

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

Merged
merged 7 commits into from
Mar 14, 2025
Merged

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