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.

Sign up for GitHub

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

fix(types): pass types through memoize properly #10567

Merged
merged 6 commits into from
Mar 5, 2024
Merged

fix(types): pass types through memoize properly #10567

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
66e5395
chore(types): pass types through memoize properly
LeoMcA Feb 21, 2024
2322633
Merge branch 'main' into fix-memoize-types
caugner Feb 28, 2024
c00822d
chore(content): reuse Translation from libs/types
caugner Feb 28, 2024
a3461ec
fix(build): please typescript
caugner Feb 28, 2024
adfbc8c
Merge branch 'main' into fix-memoize-types
LeoMcA Feb 29, 2024
212c499
Merge branch 'main' into fix-memoize-types
caugner Mar 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions build/blog.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ export async function buildBlogPosts(options: {
}
}

interface BlogPostDoc {
export interface BlogPostDoc {
url: string;
rawBody: string;
metadata: BlogPostMetadata & { locale: string };
Expand All @@ -368,7 +368,7 @@ export async function buildPost(
let $ = null;
const liveSamples: LiveSample[] = [];

[$] = await kumascript.render(document.url, {}, document as any);
[$] = await kumascript.render(document.url, {}, document);

const liveSamplePages = await kumascript.buildLiveSamplePages(
document.url,
Expand Down Expand Up @@ -449,8 +449,12 @@ export async function buildBlogFeed(options: { verbose?: boolean }) {
description: post.description,
author: [
{
name: post.author?.name || "The MDN Team",
link: post.author?.link,
name:
(typeof post.author === "string"
? post.author
: post.author?.name) || "The MDN Team",
link:
typeof post.author === "string" ? post.author : post.author?.link,
},
],
date: new Date(post.date),
Expand Down
308 changes: 157 additions & 151 deletions content/document.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import {
MEMOIZE_INVALIDATE,
} from "./utils.js";
import * as Redirect from "./redirect.js";
import { DocFrontmatter } from "../libs/types/document.js";
import { DocFrontmatter, UnbuiltDocument } from "../libs/types/document.js";
caugner marked this conversation as resolved.
Show resolved Hide resolved

export { urlToFolderPath, MEMOIZE_INVALIDATE } from "./utils.js";

Expand Down Expand Up @@ -185,173 +185,179 @@ export function getFolderPath(metadata, root: string | null = null) {
);
}

export const read = memoize((folderOrFilePath: string, ...roots: string[]) => {
if (roots.length === 0) {
roots = ROOTS;
}
let filePath: string = null;
let folder: string = null;
let root: string = null;
let locale: string = null;

if (fs.existsSync(folderOrFilePath)) {
filePath = folderOrFilePath;

// It exists, but it is sane?
if (
!(
filePath.endsWith(HTML_FILENAME) || filePath.endsWith(MARKDOWN_FILENAME)
)
) {
throw new Error(`'${filePath}' is not a HTML or Markdown file.`);
export const read = memoize(
(folderOrFilePath: string, ...roots: string[]): UnbuiltDocument => {
if (roots.length === 0) {
roots = ROOTS;
}
let filePath: string = null;
let folder: string = null;
let root: string = null;
let locale: string = null;

if (fs.existsSync(folderOrFilePath)) {
filePath = folderOrFilePath;

// It exists, but it is sane?
if (
!(
filePath.endsWith(HTML_FILENAME) ||
filePath.endsWith(MARKDOWN_FILENAME)
)
) {
throw new Error(`'${filePath}' is not a HTML or Markdown file.`);
}

root = roots.find((possibleRoot) => filePath.startsWith(possibleRoot));
if (root) {
folder = filePath
.replace(root + path.sep, "")
.replace(path.sep + HTML_FILENAME, "")
.replace(path.sep + MARKDOWN_FILENAME, "");
locale = extractLocale(filePath.replace(root + path.sep, ""));
root = roots.find((possibleRoot) => filePath.startsWith(possibleRoot));
if (root) {
folder = filePath
.replace(root + path.sep, "")
.replace(path.sep + HTML_FILENAME, "")
.replace(path.sep + MARKDOWN_FILENAME, "");
locale = extractLocale(filePath.replace(root + path.sep, ""));
} else {
// The file exists but it doesn't appear to belong to any of our roots.
// That could happen if you pass in a file that is something completely
// different not a valid file anyway.
throw new Error(
`'${filePath}' does not appear to exist in any known content roots.`
);
}
} else {
// The file exists but it doesn't appear to belong to any of our roots.
// That could happen if you pass in a file that is something completely
// different not a valid file anyway.
folder = folderOrFilePath;
for (const possibleRoot of roots) {
const possibleMarkdownFilePath = path.join(
possibleRoot,
getMarkdownPath(folder)
);
if (fs.existsSync(possibleMarkdownFilePath)) {
root = possibleRoot;
filePath = possibleMarkdownFilePath;
break;
}
const possibleHTMLFilePath = path.join(
possibleRoot,
getHTMLPath(folder)
);
if (fs.existsSync(possibleHTMLFilePath)) {
root = possibleRoot;
filePath = possibleHTMLFilePath;
break;
}
}
if (!filePath) {
return;
}
locale = extractLocale(folder);
}

if (folder.includes(" ")) {
throw new Error(
`'${filePath}' does not appear to exist in any known content roots.`
`Folder contains whitespace which is not allowed (${util.inspect(
filePath
)})`
);
}
} else {
folder = folderOrFilePath;
for (const possibleRoot of roots) {
const possibleMarkdownFilePath = path.join(
possibleRoot,
getMarkdownPath(folder)
if (folder.includes("\u200b")) {
throw new Error(
`Folder contains zero width whitespace which is not allowed (${filePath})`
);
if (fs.existsSync(possibleMarkdownFilePath)) {
root = possibleRoot;
filePath = possibleMarkdownFilePath;
break;
}
const possibleHTMLFilePath = path.join(possibleRoot, getHTMLPath(folder));
if (fs.existsSync(possibleHTMLFilePath)) {
root = possibleRoot;
filePath = possibleHTMLFilePath;
break;
}
}
if (!filePath) {
return;
}
locale = extractLocale(folder);
}

if (folder.includes(" ")) {
throw new Error(
`Folder contains whitespace which is not allowed (${util.inspect(
filePath
)})`
);
}
if (folder.includes("\u200b")) {
throw new Error(
`Folder contains zero width whitespace which is not allowed (${filePath})`
// Use Boolean() because otherwise, `isTranslated` might become `undefined`
// rather than an actual boolean value.
const isTranslated = Boolean(
CONTENT_TRANSLATED_ROOT && filePath.startsWith(CONTENT_TRANSLATED_ROOT)
);
}
// Use Boolean() because otherwise, `isTranslated` might become `undefined`
// rather than an actual boolean value.
const isTranslated = Boolean(
CONTENT_TRANSLATED_ROOT && filePath.startsWith(CONTENT_TRANSLATED_ROOT)
);

const rawContent = fs.readFileSync(filePath, "utf-8");
if (!rawContent) {
throw new Error(`${filePath} is an empty file`);
}
const rawContent = fs.readFileSync(filePath, "utf-8");
if (!rawContent) {
throw new Error(`${filePath} is an empty file`);
}

// This is very useful in CI where every page gets built. If there's an
// accidentally unresolved git conflict, that's stuck in the content,
// bail extra early.
if (
// If the document itself, is a page that explains and talks about git merge
// conflicts, i.e. a false positive, those angled brackets should be escaped
/^<<<<<<< HEAD\n/m.test(rawContent) &&
/^=======\n/m.test(rawContent) &&
/^>>>>>>>/m.test(rawContent)
) {
throw new Error(`${filePath} contains git merge conflict markers`);
}
// This is very useful in CI where every page gets built. If there's an
// accidentally unresolved git conflict, that's stuck in the content,
// bail extra early.
if (
// If the document itself, is a page that explains and talks about git merge
// conflicts, i.e. a false positive, those angled brackets should be escaped
/^<<<<<<< HEAD\n/m.test(rawContent) &&
/^=======\n/m.test(rawContent) &&
/^>>>>>>>/m.test(rawContent)
) {
throw new Error(`${filePath} contains git merge conflict markers`);
}

const {
attributes: metadata,
body: rawBody,
bodyBegin: frontMatterOffset,
} = fm<DocFrontmatter>(rawContent);
const {
attributes: metadata,
body: rawBody,
bodyBegin: frontMatterOffset,
} = fm<DocFrontmatter>(rawContent);

const url = `/${locale}/docs/${metadata.slug}`;
const url = `/${locale}/docs/${metadata.slug}`;

const isActive = ACTIVE_LOCALES.has(locale.toLowerCase());
const isActive = ACTIVE_LOCALES.has(locale.toLowerCase());

// The last-modified is always coming from the git logs. Independent of
// which root it is.
const gitHistory = getGitHistories(root, locale).get(
path.relative(root, filePath)
);
let modified = null;
let hash = null;
if (gitHistory) {
if (
gitHistory.merged &&
gitHistory.merged.modified &&
gitHistory.merged.hash
) {
modified = gitHistory.merged.modified;
hash = gitHistory.merged.hash;
} else {
modified = gitHistory.modified;
hash = gitHistory.hash;
// The last-modified is always coming from the git logs. Independent of
// which root it is.
const gitHistory = getGitHistories(root, locale).get(
path.relative(root, filePath)
);
let modified = null;
let hash = null;
if (gitHistory) {
if (
gitHistory.merged &&
gitHistory.merged.modified &&
gitHistory.merged.hash
) {
modified = gitHistory.merged.modified;
hash = gitHistory.merged.hash;
} else {
modified = gitHistory.modified;
hash = gitHistory.hash;
}
}
// Use the wiki histories for a list of legacy contributors.
const wikiHistory = getWikiHistories(root, locale).get(url);
if (!modified && wikiHistory && wikiHistory.modified) {
modified = wikiHistory.modified;
}
const fullMetadata = {
metadata: {
...metadata,
// This is our chance to record and remember which keys were actually
// dug up from the front-matter.
// It matters because the keys in front-matter are arbitrary.
// Meaning, if a document contains `foo: bar` as a front-matter key/value
// we need to take note of that and make sure we preserve that if we
// save the metadata back (e.g. fixable flaws).
frontMatterKeys: Object.keys(metadata),
locale,
popularity: getPopularities().get(url) || 0.0,
modified,
hash,
contributors: wikiHistory ? wikiHistory.contributors : [],
},
url,
};

return {
...fullMetadata,
// ...{ rawContent },
rawContent, // HTML or Markdown whole string with all the front-matter
rawBody, // HTML or Markdown string without the front-matter
isMarkdown: filePath.endsWith(MARKDOWN_FILENAME),
isTranslated,
isActive,
fileInfo: {
folder,
path: filePath,
frontMatterOffset,
root,
},
};
}
// Use the wiki histories for a list of legacy contributors.
const wikiHistory = getWikiHistories(root, locale).get(url);
if (!modified && wikiHistory && wikiHistory.modified) {
modified = wikiHistory.modified;
}
const fullMetadata = {
metadata: {
...metadata,
// This is our chance to record and remember which keys were actually
// dug up from the front-matter.
// It matters because the keys in front-matter are arbitrary.
// Meaning, if a document contains `foo: bar` as a front-matter key/value
// we need to take note of that and make sure we preserve that if we
// save the metadata back (e.g. fixable flaws).
frontMatterKeys: Object.keys(metadata),
locale,
popularity: getPopularities().get(url) || 0.0,
modified,
hash,
contributors: wikiHistory ? wikiHistory.contributors : [],
},
url,
};

return {
...fullMetadata,
// ...{ rawContent },
rawContent, // HTML or Markdown whole string with all the front-matter
rawBody, // HTML or Markdown string without the front-matter
isMarkdown: filePath.endsWith(MARKDOWN_FILENAME),
isTranslated,
isActive,
fileInfo: {
folder,
path: filePath,
frontMatterOffset,
root,
},
};
});
);

export async function update(url: string, rawBody: string, metadata) {
const folder = urlToFolderPath(url);
Expand Down
2 changes: 1 addition & 1 deletion content/translations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const LANGUAGES = new Map(
})
);

type Translation = {
export type Translation = {
Copy link
Member

Choose a reason for hiding this comment

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

We can replace the type declaration here, as there was a duplicated one in libs/types/document.ts

Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch, fixed in c00822d.

locale: string;
title: string;
native: string;
Expand Down
Loading
Loading