Skip to content

Commit

Permalink
Adds <InlinedSvg> to @rules_prerender/preact.
Browse files Browse the repository at this point in the history
This required some refactoring to perform relative path resolution using `import.meta` like `includeScript` and `inlineStyle` do. This improves performance and changes the styling story of SVGs, as they respect or don't respect certain styles based on whether they are inlined, loaded via an `<img>`, or embedded through an `<object>`, `<embed>`, or `<iframe>` (see https://css-tricks.com/scale-svg/).

I opted to use `readFileSync` for now to avoid making everything `async`. It's annoying and makes me feel dirty, but it's not the end of the world. In the future, it might be worth looking into adding some kind of "inlining" pass to the bundler, so `<InlinedSvg>` could emit an annotation which gets processed by the bundler to inline the file at the right location. Not sure if that's worth the effort right now.
  • Loading branch information
dgp1130 committed Aug 28, 2023
1 parent 7704c14 commit b0d4401
Show file tree
Hide file tree
Showing 9 changed files with 288 additions and 22 deletions.
49 changes: 43 additions & 6 deletions common/file_system.mts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { promises as fs } from 'fs';
import * as fs from 'fs';

/**
* A simplified file system interface, based on the NodeJs `fs` module with
Expand All @@ -16,24 +16,53 @@ export interface FileSystem {
mkdir(dir: string, options?: { recursive?: boolean }): Promise<void>;

/**
* Reads a file at the given path and returns it as a UTF8-encoded string or
* a binary buffer.
* Reads a file at the given path and returns it as a UTF8-encoded string.
*/
readFile(path: string, options: 'utf8'): Promise<string>;

/** Reads a file at the given path and returns it as a UTF8-encoded string. */
readFile(path: string, options: { encoding: 'utf8' }): Promise<string>;

/**
* Reads a file at the given path and returns it as a UTF8-encoded string or
* a binary buffer.
*/
readFile(path: string, options?: string | { encoding?: string }):
Promise<string | Buffer>;

/**
* Reads a file at the given path and returns it as a UTF8-encoded string.
*
* @deprecated Prefer the asynchronous {@link readFile}.
*/
readFileSync(path: string, options: 'utf8'): string;

/**
* Reads a file at the given path and returns it as a UTF8-encoded string.
*
* @deprecated Prefer the asynchronous {@link readFile}.
*/
readFileSync(path: string, options: { encoding: 'utf8' }): string;

/**
* Reads a file at the given path and returns it as a UTF8-encoded string or
* a binary buffer.
*
* @deprecated Prefer the asynchronous {@link readFile}.
*/
readFileSync(path: string, options?: string | { encoding?: string }):
string | Buffer;
}

/** A file system implemented with the Node.js `fs` APIs. */
class DiskFs implements FileSystem {
public async copyFile(src: string, dest: string): Promise<void> {
await fs.copyFile(src, dest);
await fs.promises.copyFile(src, dest);
}

public async mkdir(dir: string, options?: { recursive?: boolean }):
Promise<void> {
await fs.mkdir(dir, options);
await fs.promises.mkdir(dir, options);
}

readFile(path: string, options: 'utf8'): Promise<string>;
Expand All @@ -42,11 +71,19 @@ class DiskFs implements FileSystem {
Promise<string | Buffer>;
public async readFile(path: string, options?: 'utf8' | { encoding?: string }):
Promise<string | Buffer> {
return await fs.readFile(
return await fs.promises.readFile(
path,
options as any,
) as unknown as Promise<string | Buffer>;
}

readFileSync(path: string, options: 'utf8'): string;
readFileSync(path: string, options: { encoding: 'utf8' }): string;
readFileSync(path: string, options?: string | { encoding?: string }):
string | Buffer;
public readFileSync(path: string, options?: 'utf8' | { encoding?: string }): string | Buffer {
return fs.readFileSync(path, options as any);
}
}

/** Singleton instance of the real disk file system. */
Expand Down
21 changes: 18 additions & 3 deletions common/file_system_fake.mts
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,27 @@ export class FileSystemFake implements FileSystem {
readFile(path: string, options: { encoding: 'utf8' }): Promise<string>;
readFile(path: string, options?: string | { encoding?: string }):
Promise<string | Buffer>;
public async readFile(inputPath: string, _options?: string | { encoding?: string }): Promise<string | Buffer> {
public async readFile(
inputPath: string,
options?: string | { encoding?: string },
): Promise<string | Buffer> {
return this.readFileSync(inputPath, options);
}

readFileSync(path: string, options: 'utf8'): string;
readFileSync(path: string, options: { encoding: 'utf8' }): string;
readFileSync(path: string, options?: string | { encoding?: string }):
string | Buffer;
public readFileSync(
inputPath: string,
_options?: string | { encoding?: string },
): string | Buffer {
const filePath = path.normalize(inputPath);
const content = this.fs.get(filePath);
if (!content) {
const availableFiles = Array.from(this.fs.entries()).join('\n');
throw new Error(`File path not found. "${inputPath}" normalized to "${
const availableFiles = Array.from(this.fs.keys()).join('\n');
throw new Error(`File path not found. "${
inputPath}" normalized to "${
filePath}". Available files are:\n${availableFiles}`);
}

Expand Down
3 changes: 3 additions & 0 deletions packages/preact/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ ts_project(
srcs = ["index.mts"],
deps = [
":node_modules/preact-render-to-string",
"//:node_modules/@types/node",
"//:node_modules/preact",
# As a peer dep, we can't depend on the JS implementation of
# `rules_prerender` because it needs to be supplied at link time.
Expand All @@ -44,9 +45,11 @@ ts_project(
ts_project(
name = "preact_test_lib",
srcs = ["index_test.mts"],
testonly = True,
deps = [
":preact",
":node_modules/preact-render-to-string",
"//common:file_system_fake",
"//common/models:prerender_annotation",
"//:node_modules/@types/jasmine",
],
Expand Down
72 changes: 71 additions & 1 deletion packages/preact/index.mts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import * as path from 'path';
import { JSX, VNode, createElement } from 'preact';
import { render } from 'preact-render-to-string';
import * as rulesPrerender from 'rules_prerender';
Expand All @@ -9,7 +10,7 @@ export { PrerenderResource } from 'rules_prerender';
* The type is `VNode | VNode[]` to make the typings easier to work with, but it
* actually only supports a single {@link VNode} input of an `<html />` tag. The
* returned HTML is prefixed with `<!DOCTYPE html>`.
*
*
* @param node The {@link VNode} of an `<html />` element to render.
* @returns A {@link rulesPrerender.SafeHtml} object with input node, prefixed
* by `<!DOCTYPE html>`.
Expand Down Expand Up @@ -66,3 +67,72 @@ export function Template({ children, ...attrs }: TemplateProps = {}): VNode {
// @ts-ignore JSX types are weird AF.
return createElement('template', attrs, children);
}

/**
* Reads the linked SVG file from runfiles and returns it as a {@link VNode}.
* Note that the root node is _not_ a `<svg>`, as it must be wrapped by Preact.
*
* The linked file must be a `data` dependency of the prerender target to be
* accessible to this function.
*
* @param src Relative path from the calling file to the SVG file to inline.
* @param importMeta Must always be a literal `import.meta` at the call site.
* @param attrs Remaining attributes to apply to the returned element.
* @param fs Internal-only param, do not use.
* @returns A {@link VNode} containing the inlined SVG. Note that the root of
* the returned element is _not_ an `<svg>` element itself. The returned
* element _contains_ an `<svg>` element.
*/
export function InlinedSvg({
src,
importMeta,
fs = rulesPrerender.internalDiskFs,
...attrs
}: {
src: string,
importMeta: ImportMeta,
fs?: rulesPrerender.InternalFileSystem,
} & JSX.HTMLAttributes<HTMLElement>): VNode {
const runfiles = process.env['RUNFILES'];
if (!runfiles) throw new Error('`${RUNFILES}` not set.');

// Get the execroot relative path of the file performing the import.
// Ex: rules_prerender/bazel-out/k8-fastbuild/bin/path/to/file.mjs
const importerExecrootRelativePath =
rulesPrerender.internalExecrootRelative(
new URL(importMeta.url).pathname);

// Parse the execroot relative path to collect the workspace and relative
// path.
const { wksp, relativePath } =
rulesPrerender.internalSplitExecrootRelativePath(
importerExecrootRelativePath);

// Get the relative path to the _directory_ of the file performing the
// import.
// Ex: rules_prerender/path/to
const importerDirWkspRelativePath =
relativePath.split(path.sep).slice(0, -1).join(path.sep);

// Get the relative path to the imported file.
// Ex: path/to/some/image.svg
const importedWkspRelativePath =
path.normalize(path.join(importerDirWkspRelativePath, src));

// Get the runfiles path to the imported file.
// Ex: ${RUNFILES}/rules_prerender/path/to/some/image.svg
const importedRunfilesPath =
path.join(runfiles, wksp, importedWkspRelativePath);

// Read the SVG file. We do this synchronously to avoid async coloring. We
// should consider returning an annotation here and processing it as part of
// the build process like `includeScript` and `inlineStyle`.
const svg = fs.readFileSync(importedRunfilesPath, 'utf8');

// Convert the SVG text into a Preact VNode. This unfortunately requires a
// wrapper element.
return createElement('div', {
...attrs,
dangerouslySetInnerHTML: { __html: svg },
});
}
40 changes: 39 additions & 1 deletion packages/preact/index_test.mts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { createElement } from 'preact';
import { render } from 'preact-render-to-string';
import { renderToHtml, includeScript, inlineStyle, Template } from './index.mjs';
import { InlinedSvg, Template, renderToHtml, includeScript, inlineStyle } from './index.mjs';
import { serialize } from '../../common/models/prerender_annotation.mjs';
import { FileSystemFake } from '../../common/file_system_fake.mjs';

describe('preact', () => {
describe('renderToHtml()', () => {
Expand Down Expand Up @@ -135,4 +136,41 @@ describe('preact', () => {
expect(() => Template({ notAnAttribute: 'test' })).not.toThrow();
});
});

describe('InlinedSvg()', () => {
const actualRunfiles = process.env['RUNFILES'];
afterEach(() => {
process.env['RUNFILES'] = actualRunfiles;
});

it('inlines an `<svg>` element', () => {
process.env['RUNFILES'] = 'MOCK_RUNFILES_PATH';
const fs = FileSystemFake.of({
'MOCK_RUNFILES_PATH/my_wksp/path/to/pkg/image.svg': '<svg></svg>',
});

const html = render(InlinedSvg({
src: './image.svg',
importMeta: {
url: 'file:///bazel/.../execroot/my_wksp/bazel-out/k8-opt/bin/path/to/pkg/prerender.mjs',
},
fs,
}));

expect(html).toContain('<svg></svg>');
});

it('throws an error when `$RUNFILES` is not set', () => {
delete process.env['RUNFILES'];
const fs = FileSystemFake.of({});

expect(() => render(InlinedSvg({
src: './image.svg',
importMeta: {
url: 'file:///bazel/.../execroot/my_wksp/bazel-out/k8-opt/bin/path/to/pkg/prerender.mjs',
},
fs,
}))).toThrowError(/`\${RUNFILES}` not set/);
});
});
});
1 change: 1 addition & 0 deletions packages/rules_prerender/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ ts_project(
deps = [
":scripts",
":styles",
"//common:file_system",
"//common/models:prerender_resource",
"//common/safe_html",
"//common/safe_html:unsafe_html",
Expand Down
10 changes: 9 additions & 1 deletion packages/rules_prerender/index.mts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
/** @fileoverview Re-exports public symbols. */

export { PrerenderResource } from '../../common/models/prerender_resource.mjs';
export {
includeScript,
includeScriptAnnotation as internalIncludeScriptAnnotation,
Expand All @@ -15,6 +14,15 @@ export {
setMap as internalSetInlineStyleMap,
resetMapForTesting as internalResetInlineStyleMapForTesting,
} from './inline_style_map.mjs';
export {
execrootRelative as internalExecrootRelative,
parseExecrootRelativePath as internalSplitExecrootRelativePath,
} from './paths.mjs';

export {
FileSystem as InternalFileSystem,
diskFs as internalDiskFs,
} from '../../common/file_system.mjs';
export { PrerenderResource } from '../../common/models/prerender_resource.mjs';
export { SafeHtml, isSafeHtml } from '../../common/safe_html/safe_html.mjs';
export { unsafeTreatStringAsSafeHtml } from '../../common/safe_html/unsafe_html.mjs';
50 changes: 42 additions & 8 deletions packages/rules_prerender/paths.mts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@ import * as path from 'path';

/** Converts the given absolute file path to a workspace-relative file path. */
export function wkspRelative(filePath: string): string {
const execrootRelativePath = execrootRelative(filePath);
const { relativePath } = parseExecrootRelativePath(execrootRelativePath);
return relativePath;
}

/** Converts the given absolute file path to an execroot-relative file path. */
export function execrootRelative(filePath: string): string {
const execrootDir = `${path.sep}execroot${path.sep}`;
if (!filePath.includes(execrootDir)) {
throw new Error(`Path not in the Bazel workspace: "${filePath}".`);
Expand All @@ -10,17 +17,44 @@ export function wkspRelative(filePath: string): string {
// Strip everything up to the first `/execroot/` directory. User code could
// name their own directories `execroot` so we only take up to the first.
const execrootStart = filePath.indexOf(execrootDir) + execrootDir.length;
const execrootRelativePath = filePath.slice(execrootStart);
return filePath.slice(execrootStart);
}

export type Configuration =
| `k8-${string}`
| `win-${string}`
| `darwin-${string}`;

// Drop leading `wksp/bazel-out/${cfg}/bin/` directories. `@aspect_rules_js`
// requires all the files to be in the bin directory anyways, so we know
// those paths will always need to be removed. If not, then the user is
// likely depending on a source file directly and needs to `copy_to_bin()`.
const [ _wksp, bazelOut, _cfg, binOrGenfiles, ...relativeDirs ] =
/** A parsed format of an execroot-relative path. */
export interface ExecrootRelativePath {
wksp: string,
cfg: Configuration,
binOrGenfiles: 'bin' | 'genfiles',
relativePath: string,
}

/** Parses an execroot relative path and returns the parsed format. */
export function parseExecrootRelativePath(execrootRelativePath: string):
ExecrootRelativePath {
// `@aspect_rules_js` requires all the files to be in the bin directory
// anyways, so we know those paths will always be present. If not, then the
// user is likely depending on a source file directly and needs to
// `copy_to_bin()`.
const [ wksp, bazelOut, cfg, binOrGenfiles, ...relativeDirs ] =
execrootRelativePath.split(path.sep);
if (bazelOut !== 'bazel-out' || (binOrGenfiles !== 'bin' && binOrGenfiles !== 'genfiles')) {
if (bazelOut !== 'bazel-out') {
throw new Error(`Path not in \`bazel-out\`, did you forget to \`copy_to_bin()\`? "${execrootRelativePath}"`);
}
if (binOrGenfiles !== 'bin' && binOrGenfiles !== 'genfiles') {
throw new Error(`Path not in artifact root, did you forget to \`copy_to_bin()\`? "${execrootRelativePath}"`);
}

return relativeDirs.join(path.sep);
return {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
wksp: wksp!,
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
cfg: cfg! as Configuration,
binOrGenfiles,
relativePath: relativeDirs.join(path.sep),
};
}
Loading

0 comments on commit b0d4401

Please sign in to comment.