fix(bcd): show previous version in range when feature was removed

[danielhjacobs]

Summary

Fixes #6667

Problem

The version range showed the version where support was removed as well, which was confusing.

Solution

Get version before that version using a helper function, and show that version number instead.

---

Screenshots

Before

https://developer.mozilla.org/en-US/docs/Web/API/DocumentType#browser_compatibility

After

http://localhost:3000/en-US/docs/Web/API/DocumentType#browser_compatibility

[foolip]

That looks much better!

[caugner]

LGTM, just two nits (I think).

[caugner]

Thank you @danielhjacobs! 🎉

[caugner]

Hm, for some reason it doesn't work on prod (although deployed): https://developer.mozilla.org/en-US/docs/Web/API/DocumentType#browser_compatibility 🤔

[caugner]

Hm, for some reason it doesn't work on prod (although deployed): https://developer.mozilla.org/en-US/docs/Web/API/DocumentType#browser_compatibility 🤔

It looks like browser.releases only has the last 3 entries on prod:

That's because retired releases are not added to the bcd.json:

yari/build/cli.ts

Lines 199 to 211 in 9a46d54

	// The BCD data object contains a bunch of data we don't need in the
	// React component that loads the `bcd.json` file and displays it.
	// The `.releases` block contains information about browsers (e.g
	// release dates) and that part has already been extracted and put
	// next to each version number where appropriate.
	// Therefore, we strip out all "retired" releases.
	if (key === "releases") {
	return Object.fromEntries(
	Object.entries(value as bcd.ReleaseStatement).filter(
	([, v]) => v.status !== "retired"
	)
	);
	}

edit: And the BCD data itself is extracted here:

yari/build/document-extractor.ts

Lines 424 to 507 in 837e5d0

	function _buildSpecialBCDSection(): [BCDSection] {
	// First extract a map of all release data, keyed by (normalized) browser
	// name and the versions.
	// You'll have a map that looks like this:
	//
	// 'chrome_android': {
	// '28': {
	// release_date: '2012-06-01',
	// release_notes: '...',
	// ...
	//
	// The reason we extract this to a locally scoped map, is so we can
	// use it to augment the `__compat` blocks for the latest version
	// when (if known) it was added.
	const browserReleaseData = new Map();
	for (const [name, browser] of Object.entries(browsers)) {
	const releaseData = new Map();
	for (const [version, data] of Object.entries(browser.releases || [])) {
	if (data) {
	releaseData.set(version, data);
	}
	}
	browserReleaseData.set(name, releaseData);
	}
	for (const block of _extractCompatBlocks(data)) {
	for (const [browser, originalInfo] of Object.entries(block.support)) {
	// `originalInfo` here will be one of the following:
	// - a single simple_support_statement:
	// { version_added: 42 }
	// - an array of simple_support_statements:
	// [ { version_added: 42 }, { prefix: '-moz', version_added: 35 } ]
	//
	// Standardize the first version to an array of one, so we don't have
	// to deal with two different forms below
	const infos: SimpleSupportStatementWithReleaseDate[] = Array.isArray(
	originalInfo
	)
	? originalInfo
	: [originalInfo];
	for (const infoEntry of infos) {
	const added =
	typeof infoEntry.version_added === "string" &&
	infoEntry.version_added.startsWith("≤")
	? infoEntry.version_added.slice(1)
	: infoEntry.version_added;
	if (browserReleaseData.has(browser)) {
	if (browserReleaseData.get(browser).has(added)) {
	infoEntry.release_date = browserReleaseData
	.get(browser)
	.get(added).release_date;
	}
	}
	}
	infos.sort((a, b) =>
	_compareVersions(_getFirstVersion(b), _getFirstVersion(a))
	);
	block.support[browser] = infos;
	}
	}
	if (hasMultipleQueries) {
	title = query;
	id = query;
	isH3 = true;
	}
	return [
	{
	type: "browser_compatibility",
	value: {
	title,
	id,
	isH3,
	data,
	query,
	browsers,
	},
	},
	];
	}

[danielhjacobs]

yari/build/document-extractor.ts

Lines 481 to 483 in 837e5d0

	infos.sort((a, b) =>
	_compareVersions(_getFirstVersion(b), _getFirstVersion(a))
	);

Well that function seems useful, could avoid the compare-versions dependency by re-using it.

I assume the right thing to do then would be to add version_supported_last (as suggested in https://bugzilla.mozilla.org/show_bug.cgi?id=1417902#c1) to infoEntry around here:

yari/build/document-extractor.ts

Lines 474 to 476 in 837e5d0

	infoEntry.release_date = browserReleaseData
	.get(browser)
	.get(added).release_date;

Also adding version_supported_last?: string here:

yari/build/document-extractor.ts

Lines 14 to 17 in 837e5d0

	interface SimpleSupportStatementWithReleaseDate
	extends bcd.SimpleSupportStatement {
	release_date?: string;
	}

At that part of the code, are all the browser releases available in browsers?

If so, that would the right place for this function. I assume browsers[browser] would be the BrowserStatement and infoEntry?.version_removed ?? null would be the VersionValue, but I'm not certain:

yari/client/src/document/ingredients/browser-compatibility-table/utils.ts

Lines 197 to 212 in 9a46d54

	export function getPreviousVersion(
	version: BCD.VersionValue,
	browser: BCD.BrowserStatement
	): BCD.VersionValue {
	if (browser && typeof version === "string") {
	const browserVersions = Object.keys(browser["releases"]).sort(
	compareVersions
	);
	const currentVersionIndex = browserVersions.indexOf(version);
	if (currentVersionIndex > 0) {
	return browserVersions[currentVersionIndex - 1];
	}
	}
	return version;
	}

Then switching this to const removed = currentSupport?.version_supported_last ?? null; (possibly changing the variable name there and where it's used to lastSupported):

yari/client/src/document/ingredients/browser-compatibility-table/feature-row.tsx

Lines 139 to 142 in 9a46d54

	const removed = getPreviousVersion(
	currentSupport?.version_removed ?? null,
	browser
	);

[danielhjacobs]

I do notice you're currently editing that part of the codebase in #7612

[danielhjacobs]

Opened #7614