Skip to content

Commit

Permalink
feat(docs): allow audio (mp3/ogg), video (mp4/webm) and font (woff2) …
Browse files Browse the repository at this point in the history
…attachments (#7605)

These file types can now be stored next to a Markdown file to be used in live samples.

In the future, only two steps are necessary to add a file extension/type:
1. The file extension (and its mime type) must be added to `libs/constants` and `client/src/setupProxy.js`.
2. A module must be declared in `{server,ssr}/react-app.d.ts`.

Notes:
- The `Image` module is now called `FileAttachment`, e.g. `FileAttachment.findByUrl()`.
- A new npm script `test:libs` was added to run all unit tests in `libs/`.
  • Loading branch information
caugner authored Jun 2, 2023
1 parent cd5e710 commit 73f8dbc
Show file tree
Hide file tree
Showing 19 changed files with 320 additions and 45 deletions.
3 changes: 3 additions & 0 deletions .github/workflows/testing.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ jobs:
- name: Unit testing client
run: yarn test:client

- name: Unit testing libs
run: yarn test:libs

- name: Build and start server
id: server
env:
Expand Down
12 changes: 6 additions & 6 deletions build/check-images.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import path from "node:path";

import imagesize from "image-size";

import { Document, Image } from "../content/index.js";
import { Document, FileAttachment } from "../content/index.js";
import { FLAW_LEVELS, DEFAULT_LOCALE } from "../libs/constants/index.js";
import { findMatchesInText } from "./matches-in-text.js";
import * as cheerio from "cheerio";
Expand Down Expand Up @@ -140,12 +140,12 @@ export function checkImageReferences(
// but all our images are going to be static.
finalSrc = absoluteURL.pathname;
// We can use the `finalSrc` to look up and find the image independent
// of the correct case because `Image.findByURL` operates case
// of the correct case because `FileAttachment.findByURL` operates case
// insensitively.

// What follows uses the same algorithm as Image.findByURLWithFallback
// What follows uses the same algorithm as FileAttachment.findByURLWithFallback
// but only adds a filePath if it exists for the DEFAULT_LOCALE
const filePath = Image.findByURL(finalSrc);
const filePath = FileAttachment.findByURL(finalSrc);
let enUSFallback = false;
if (
!filePath &&
Expand All @@ -156,7 +156,7 @@ export function checkImageReferences(
new RegExp(`^/${doc.locale}/`, "i"),
`/${DEFAULT_LOCALE}/`
);
if (Image.findByURL(enUSFinalSrc)) {
if (FileAttachment.findByURL(enUSFinalSrc)) {
// Use the en-US src instead
finalSrc = enUSFinalSrc;
// Note that this `<img src="...">` value can work if you use the
Expand Down Expand Up @@ -366,7 +366,7 @@ export function checkImageWidths(
);
}
} else if (!imgSrc.includes("://") && imgSrc.startsWith("/")) {
const filePath = Image.findByURLWithFallback(imgSrc);
const filePath = FileAttachment.findByURLWithFallback(imgSrc);
if (filePath) {
const dimensions = sizeOf(filePath);
img.attr("width", `${dimensions.width}`);
Expand Down
6 changes: 3 additions & 3 deletions build/flaws/broken-links.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import fs from "node:fs";
import { fromMarkdown } from "mdast-util-from-markdown";
import { visit } from "unist-util-visit";

import { Document, Redirect, Image } from "../../content/index.js";
import { Document, Redirect, FileAttachment } from "../../content/index.js";
import { findMatchesInText } from "../matches-in-text.js";
import {
DEFAULT_LOCALE,
Expand Down Expand Up @@ -277,8 +277,8 @@ export function getBrokenLinksFlaws(
const absoluteURL = new URL(href, "http://www.example.com");
const found = Document.findByURL(hrefNormalized);
if (!found) {
// Before we give up, check if it's an image.
if (!Image.findByURLWithFallback(hrefNormalized)) {
// Before we give up, check if it's an attachment.
if (!FileAttachment.findByURLWithFallback(hrefNormalized)) {
// Even if it's a redirect, it's still a flaw, but it'll be nice to
// know what it *should* be.
const resolved = Redirect.resolve(hrefNormalized);
Expand Down
6 changes: 3 additions & 3 deletions build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import LANGUAGES_RAW from "../libs/languages/index.js";
import { safeDecodeURIComponent } from "../kumascript/src/api/util.js";
import { wrapTables } from "./wrap-tables.js";
import {
getAdjacentImages,
getAdjacentFileAttachments,
injectLoadingLazyAttributes,
injectNoTranslate,
makeTOC,
Expand Down Expand Up @@ -382,8 +382,8 @@ export async function buildDocument(
// The checkImageReferences() does 2 things. Checks image *references* and
// it returns which images it checked. But we'll need to complement any
// other images in the folder.
getAdjacentImages(path.dirname(document.fileInfo.path)).forEach((fp) =>
fileAttachments.add(fp)
getAdjacentFileAttachments(path.dirname(document.fileInfo.path)).forEach(
(fp) => fileAttachments.add(fp)
);

// Check the img tags for possible flaws and possible build-time rewrites
Expand Down
26 changes: 13 additions & 13 deletions build/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,11 @@ import imageminSvgo from "imagemin-svgo";
import { rgPath } from "@vscode/ripgrep";
import sanitizeFilename from "sanitize-filename";

import { VALID_MIME_TYPES } from "../libs/constants/index.js";
import { Image } from "../content/index.js";
import {
ANY_ATTACHMENT_REGEXP,
VALID_MIME_TYPES,
} from "../libs/constants/index.js";
import { FileAttachment } from "../content/index.js";
import { spawnSync } from "node:child_process";
import { BLOG_ROOT } from "../libs/env/index.js";

Expand Down Expand Up @@ -184,15 +187,12 @@ export function splitSections(rawHTML) {
*
* @param {Document} document
*/
export function getAdjacentImages(documentDirectory) {
export function getAdjacentFileAttachments(documentDirectory: string) {
const dirents = fs.readdirSync(documentDirectory, { withFileTypes: true });
return dirents
.filter((dirent) => {
// This needs to match what we do in filecheck/checker.py
return (
!dirent.isDirectory() &&
/\.(png|jpeg|jpg|gif|svg|webp)$/i.test(dirent.name)
);
return !dirent.isDirectory() && ANY_ATTACHMENT_REGEXP.test(dirent.name);
})
.map((dirent) => path.join(documentDirectory, dirent.name));
}
Expand Down Expand Up @@ -249,21 +249,21 @@ export function postLocalFileLinks($, doc) {
const href = element.attribs.href;

// This test is merely here to quickly bail if there's no hope to find the
// image as a local file link. There are a LOT of hyperlinks throughout
// the content and this simple if statement means we can skip 99% of the
// links, so it's presumed to be worth it.
// file attachment as a local file link. There are a LOT of hyperlinks
// throughout the content and this simple if statement means we can skip 99%
// of the links, so it's presumed to be worth it.
if (
!href ||
/^(\/|\.\.|http|#|mailto:|about:|ftp:|news:|irc:|ftp:)/i.test(href)
) {
return;
}
// There are a lot of links that don't match. E.g. `<a href="SubPage">`
// So we'll look-up a lot "false positives" that are not images.
// So we'll look-up a lot "false positives" that are not file attachments.
// Thankfully, this lookup is fast.
const url = `${doc.mdn_url}/${href}`;
const image = Image.findByURLWithFallback(url);
if (image) {
const fileAttachment = FileAttachment.findByURLWithFallback(url);
if (fileAttachment) {
$(element).attr("href", url);
}
});
Expand Down
25 changes: 25 additions & 0 deletions client/src/react-app.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,41 @@ declare module "*.jpeg" {
export default src;
}

declare module "*.mp3" {
const src: string;
export default src;
}

declare module "*.mp4" {
const src: string;
export default src;
}

declare module "*.ogg" {
const src: string;
export default src;
}

declare module "*.png" {
const src: string;
export default src;
}

declare module "*.webm" {
const src: string;
export default src;
}

declare module "*.webp" {
const src: string;
export default src;
}

declare module "*.woff2" {
const src: string;
export default src;
}

declare module "*.svg" {
import * as React from "react";

Expand Down
4 changes: 2 additions & 2 deletions client/src/setupProxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ function config(app) {
app.use("/_+(flaws|translations|open|document)", proxy);
// E.g. search-index.json or index.json
app.use("**/*.json", proxy);
// This has to match what we do in server/index.js in the catchall handler
app.use("**/*.(png|webp|gif|jpe?g|svg)", proxy);
// Always update libs/constant/index.js when adding/removing extensions!
app.use(`**/*.(gif|jpeg|jpg|mp3|mp4|ogg|png|svg|webm|webp|woff2)`, proxy);
// All those root-level images like /favicon-48x48.png
app.use("/*.(png|webp|gif|jpe?g|svg)", proxy);
}
Expand Down
4 changes: 3 additions & 1 deletion cloud-function/src/app.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import express, { Request, Response } from "express";
import { Router } from "express";

import { ANY_ATTACHMENT_EXT } from "./internal/constants/index.js";

import { Origin } from "./env.js";
import { proxyContent } from "./handlers/proxy-content.js";
import { proxyKevel } from "./handlers/proxy-kevel.js";
Expand Down Expand Up @@ -48,7 +50,7 @@ router.get(
proxyContent
);
router.get(
"/[^/]+/docs/*/*.(png|jpeg|jpg|gif|svg|webp)",
`/[^/]+/docs/*/*.(${ANY_ATTACHMENT_EXT.join("|")})`,
requireOrigin(Origin.main, Origin.liveSamples),
resolveIndexHTML,
proxyContent
Expand Down
13 changes: 11 additions & 2 deletions cloud-function/src/utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import { Request, Response } from "express";

import {
ANY_ATTACHMENT_EXT,
createRegExpFromExtensions,
} from "./internal/constants/index.js";

import { DEFAULT_COUNTRY } from "./constants.js";

export function getRequestCountry(req: Request): string {
Expand Down Expand Up @@ -45,8 +50,12 @@ export function isLiveSampleURL(url: string) {

// These are the only extensions in client/build/*/docs/*.
// `find client/build -type f | grep docs | xargs basename | sed 's/.*\.\([^.]*\)$/\1/' | sort | uniq`
const ASSET_REGEXP = /\.(gif|html|jpeg|jpg|json|png|svg|txt|xml)$/i;
const TEXT_EXT = ["html", "json", "svg", "txt", "xml"];
const ANY_ATTACHMENT_REGEXP = createRegExpFromExtensions(
...ANY_ATTACHMENT_EXT,
...TEXT_EXT
);

export function isAsset(url: string) {
return ASSET_REGEXP.test(url);
return ANY_ATTACHMENT_REGEXP.test(url);
}
44 changes: 41 additions & 3 deletions content/image.ts → content/file-attachment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,52 @@ import { readChunkSync } from "read-chunk";
import imageType from "image-type";
import isSvg from "is-svg";

import { DEFAULT_LOCALE } from "../libs/constants/index.js";
import {
ANY_IMAGE_EXT,
AUDIO_EXT,
DEFAULT_LOCALE,
FONT_EXT,
VIDEO_EXT,
createRegExpFromExtensions,
} from "../libs/constants/index.js";
import { ROOTS } from "../libs/env/index.js";
import { memoize, slugToFolder } from "./utils.js";

function isImage(filePath: string) {
function isFileAttachment(filePath: string) {
if (fs.statSync(filePath).isDirectory()) {
return false;
}

return (
isAudio(filePath) ||
isFont(filePath) ||
isVideo(filePath) ||
isImage(filePath)
);
}

const AUDIO_FILE_REGEXP = createRegExpFromExtensions(...AUDIO_EXT);
const FONT_FILE_REGEXP = createRegExpFromExtensions(...FONT_EXT);
const VIDEO_FILE_REGEXP = createRegExpFromExtensions(...VIDEO_EXT);
const IMAGE_FILE_REGEXP = createRegExpFromExtensions(...ANY_IMAGE_EXT);

function isAudio(filePath: string) {
return AUDIO_FILE_REGEXP.test(filePath);
}

function isFont(filePath: string) {
return FONT_FILE_REGEXP.test(filePath);
}

function isVideo(filePath: string) {
return VIDEO_FILE_REGEXP.test(filePath);
}

function isImage(filePath: string) {
if (!IMAGE_FILE_REGEXP.test(filePath)) {
return false;
}

if (filePath.toLowerCase().endsWith(".svg")) {
return isSvg(fs.readFileSync(filePath, "utf-8"));
}
Expand All @@ -37,7 +75,7 @@ function urlToFilePath(url: string) {

const find = memoize((relativePath: string) => {
return ROOTS.map((root) => path.join(root, relativePath)).find(
(filePath) => fs.existsSync(filePath) && isImage(filePath)
(filePath) => fs.existsSync(filePath) && isFileAttachment(filePath)
);
});

Expand Down
2 changes: 1 addition & 1 deletion content/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ export * as Document from "./document.js";
export * as Translation from "./translation.js";
export { getPopularities } from "./popularities.js";
export * as Redirect from "./redirect.js";
export * as Image from "./image.js";
export * as FileAttachment from "./file-attachment.js";
export {
buildURL,
memoize,
Expand Down
Loading

0 comments on commit 73f8dbc

Please sign in to comment.