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

optimize how we set the shortcut icon #3621

Merged
merged 7 commits into from
Apr 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Binary file added client/public/favicon-48x48.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
7 changes: 7 additions & 0 deletions client/public/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width, initial-scale=1" />

<!--
Note that our build process will rewrite this 'href' value to a copy
of the file that has a hash in it.
-->

<link rel="icon" href="%PUBLIC_URL%/favicon-48x48.png" />

<!-- third-generation iPad with high-resolution Retina display: -->
<link
rel="apple-touch-icon-precomposed"
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"fiori:build": "cd client && build-storybook",
"fiori:start": "cd client && start-storybook -p 6006",
"md": "node markdown/cli.js",
"prepare-build": "yarn build:client && yarn build:ssr && yarn tool google-analytics-code && yarn tool spas && yarn tool gather-git-history",
"prepare-build": "yarn build:client && yarn build:ssr && yarn tool optimize-client-build && yarn tool google-analytics-code && yarn tool spas && yarn tool gather-git-history",
"prettier-check": "prettier --check .",
"prettier-format": "prettier --write .",
"start": "(test -f client/build/index.html || yarn build:client) && (test -f ssr/dist/main.js || yarn build:ssr) && (test -d client/build/en-us/_spas || yarn tool spas) && nf -j Procfile.start start",
Expand Down Expand Up @@ -80,6 +80,7 @@
"js-yaml": "4.1.0",
"loglevel": "^1.7.1",
"lru-cache": "^6.0.0",
"md5-file": "5.0.0",
"mdn-data": "2.0.18",
"open": "^8.0.6",
"open-editor": "3.0.0",
Expand Down
7 changes: 7 additions & 0 deletions testing/tests/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,13 @@ test("content built foo page", () => {
const html = fs.readFileSync(htmlFile, "utf-8");
const $ = cheerio.load(html);

// Check that the favicon works and resolves
const faviconHref = $('link[rel="icon"]').attr("href");
// The faviconHref is a URL so to check that it exists on disk we need to
// strip the leading / and join that with the root of the build.
const faviconFile = path.join(buildRoot, faviconHref.slice(1));
expect(fs.existsSync(faviconFile)).toBeTruthy();

expect($('meta[name="description"]').attr("content")).toBe(
"This becomes the summary."
);
Expand Down
29 changes: 29 additions & 0 deletions tool/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const {
GOOGLE_ANALYTICS_DEBUG,
} = require("../build/constants");
const { runMakePopularitiesFile } = require("./popularities");
const { runOptimizeClientBuild } = require("./optimize-client-build");
const kumascript = require("../kumascript");

const PORT = parseInt(process.env.SERVER_PORT || "5000");
Expand Down Expand Up @@ -879,6 +880,34 @@ if (Mozilla && !Mozilla.dntEnabled()) {
`modified: ${countModified} | no-change: ${countNoChange} | skipped: ${countSkipped} | total: ${countTotal}`
);
})
)

.command(
"optimize-client-build",
"After the client code has been built there are things to do that react-scripts can't."
)
.argument("<buildroot>", "directory where react-scripts built", {
default: path.join("client", "build"),
})
.action(
tryOrExit(async ({ args, options, logger }) => {
const { buildroot } = args;
const { results } = await runOptimizeClientBuild(buildroot);
if (options.verbose) {
for (const result of results) {
logger.info(`${result.filePath} -> ${result.hashedHref}`);
}
} else {
logger.info(
chalk.green(
`Hashed ${results.length} files in ${path.join(
buildroot,
"index.html"
)}`
)
);
}
})
);

program.run();
94 changes: 94 additions & 0 deletions tool/optimize-client-build.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/**
* This script does all the necessary things the `yarn client:build`
* (react-scripts) can't do.
*
*/
const fs = require("fs");
const path = require("path");

const cheerio = require("cheerio");
const md5File = require("md5-file");

async function runOptimizeClientBuild(buildRoot) {
const indexHtmlFilePath = path.join(buildRoot, "index.html");
const indexHtml = fs.readFileSync(indexHtmlFilePath, "utf-8");

const results = [];

// For every favicon referred there, change it to a file URL that
// has a hash in it.
const $ = cheerio.load(indexHtml);
$("link[rel]").each((i, element) => {
const href = element.attribs.href;
if (!href) {
return;
}
const rel = element.attribs.rel;
if (
![
"icon",
"shortcut icon",
"apple-touch-icon-precomposed",
"manifest",
].includes(rel)
) {
return;
}
// If this script is, for some reason, already run before we can
// bail if it looks like the href already is hashed.
if (/\.[a-f0-9]{8}\./.test(href)) {
console.warn(`Looks like ${href} is already hashed`);
return;
}
const filePath = hrefToFilePath(buildRoot, href);
if (!filePath || !fs.existsSync(filePath)) {
console.warn(`Unable to turn '${href}' into a valid file path`);
return;
}
// 8 because that's what react-scripts (which uses webpack somehow)
// uses to create those `build/static/**/*` files it builds.
const hash = md5File.sync(filePath).slice(0, 8);
const extName = path.extname(filePath);
const splitName = filePath.split(extName);
const hashedFilePath = `${splitName[0]}.${hash}${extName}`;
fs.copyFileSync(filePath, hashedFilePath);
const hashedHref = filePathToHref(buildRoot, hashedFilePath);
results.push({
filePath,
href,
hashedHref,
hashedFilePath,
});
});

if (results.length > 0) {
// It clearly hashed some files. Let's update the HTML!
let newIndexHtml = indexHtml;
for (const { href, hashedHref } of results) {
newIndexHtml = newIndexHtml.replace(
new RegExp(`href="${href}"`),
`href="${hashedHref}"`
);
}
fs.writeFileSync(indexHtmlFilePath, newIndexHtml, "utf-8");
}

return { results };
}

// Turn 'C:\Path\to\client\build\favicon.ico' to '/favicon.ico'
function filePathToHref(root, filePath) {
return `${filePath.replace(root, "").replace(path.sep, "/")}`;
}

// Turn '/favicon.ico' to 'C:\Path\to\client\build\favicon.ico'
function hrefToFilePath(root, href) {
// The href is always expected to start with a `/` which is part of a
// URL and not a file path.
const pathname = new URL(href, "http://localhost.example").pathname;
if (pathname.startsWith("/")) {
return path.join(root, pathname.slice(1).replace(/\//g, path.sep));
}
}

module.exports = { runOptimizeClientBuild };
5 changes: 5 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -13144,6 +13144,11 @@ mathml-tag-names@^2.1.3:
resolved "https://registry.yarnpkg.com/mathml-tag-names/-/mathml-tag-names-2.1.3.tgz#4ddadd67308e780cf16a47685878ee27b736a0a3"
integrity sha512-APMBEanjybaPzUrfqU0IMU5I0AswKMH7k8OTLs0vvV4KZpExkTkY87nR/zpbuTPj+gARop7aGUbl11pnDfW6xg==

md5-file@5.0.0:
version "5.0.0"
resolved "https://registry.yarnpkg.com/md5-file/-/md5-file-5.0.0.tgz#e519f631feca9c39e7f9ea1780b63c4745012e20"
integrity sha512-xbEFXCYVWrSx/gEKS1VPlg84h/4L20znVIulKw6kMfmBUAZNAnF00eczz9ICMl+/hjQGo5KSXRxbL/47X3rmMw==

md5.js@^1.3.4:
version "1.3.5"
resolved "https://registry.yarnpkg.com/md5.js/-/md5.js-1.3.5.tgz#b5d07b8e3216e3e27cd728d72f70d1e6a342005f"
Expand Down