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

enhance(bcd): Remove redundant getCurrentSupport() #10140

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

queengooborg
Copy link
Collaborator

@queengooborg queengooborg commented Dec 6, 2023

Summary

This PR removes the getCurrentSupport() helper function from the BCD renderer.

Warning: requires a PR to land in bcd-utils to function: mdn/bcd-utils#29

Problem

About a year ago, a change landed in BCD to automatically order the support statements in a way that is nearly identical to how getCurrentSupport() searches through statements. It is redundant to define this function, since it would mean we're duplicating work.

Solution

Since it is redundant to perform work twice, the solution is simple: just remove the function, and get the first statement in the array that BCD sorts.


How did you test this change?

I spent a LOT of time unlinking Yari from @mdn/bcd-utils-api (see #10186). Some things were still broken (they've been fixed since due to updates to BCD), I confirmed it was picking the correct statement:

developer.mozilla.org (history open to show the statements it is choosing from):
image

local:
image

@queengooborg queengooborg requested a review from a team as a code owner December 6, 2023 09:06
@github-actions github-actions bot added the idle label Feb 7, 2024
@argl argl added needs decision from engineering browser-compat issues related to the browser compatibility data tables (BCD) and removed idle labels Mar 20, 2024
@argl
Copy link
Contributor

argl commented Mar 20, 2024

The dependent PR in bcd-utils has been merged.

@caugner
Copy link
Contributor

caugner commented Mar 28, 2024

a change landed in BCD to automatically order the support statements in a way that is nearly identical to how getCurrentSupport() searches through statements

@queengooborg Can you clarify what you mean by "nearly identical"? Are there differences that would lead to changes in what is shown in BCD tables? (Besides, we could keep getCurrentSupport(support) and implement it as getFirst(support) with a comment to make the intention clearer where it's called.)

@queengooborg
Copy link
Collaborator Author

Can you clarify what you mean by "nearly identical"? Are there differences that would lead to changes in what is shown in BCD tables?

When I say "nearly identical", I mean that BCD organizes its statements in a way where the first result in an array of statements would be the same as what getCurrentSupport() would have picked. However, BCD's sorting has been implemented and refined independently from Yari, so there are bound to be some minor differences in the ordering of other statements. You can find the code that orders statements in BCD here: https://github.com/mdn/browser-compat-data/blob/main/scripts/lib/compare-statements.ts

@github-actions github-actions bot added the idle label Apr 27, 2024
@github-actions github-actions bot removed the idle label Aug 16, 2024
@github-actions github-actions bot added the idle label Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
browser-compat issues related to the browser compatibility data tables (BCD) idle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants