Skip to content

Commit

Permalink
Allows undefined inline style map in inlineStyle().
Browse files Browse the repository at this point in the history
Test use cases which use `:%{component}_prerender_for_test` from `prerender_component()` directly call prerender JS from a test. While we _theoretically_ have the knowledge necessary to inject the inline style map, there is no good opportunity to load it. It could be done in a hacky way by including a `ts_library(runtime_deps = [...])` (generally considered bad practice and not supported by `ts_project()`) or rewriting the user's source files in the `:%{component}_prerender_for_test` target (would break imports in a non-obvious way).

Ultimately such test use cases don't really care about the inline style map, since they shouldn't be asserting against it at all. The simplest way to support them is to just allow the inline style map to not be set. This could result in future bugs where the map isn't set when it should be, but I expect usage to pretty stable and since it is an internal implementation detail, only I should be able to make that mistake, not every user of `rules_prerender`.
  • Loading branch information
dgp1130 committed May 1, 2022
1 parent 04e4772 commit a720fff
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 5 deletions.
3 changes: 1 addition & 2 deletions packages/rules_prerender/inline_style_map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@
let map: ReadonlyMap<string, string> | undefined;

/** Returns the inline style map. */
export function getMap(): ReadonlyMap<string, string> {
if (!map) throw new Error('Inline style map not set.');
export function getMap(): ReadonlyMap<string, string> | undefined {
return map;
}

Expand Down
10 changes: 7 additions & 3 deletions packages/rules_prerender/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,17 @@ export function includeStyle(path: string): string {
*/
export function inlineStyle(importPath: string): string {
// Look up the import path in the inline style map to get its actual file
// path on disk.
// path on disk. This will always exist when executed as part of the
// renderer, however tests which directly call components won't set the
// inline style map and it won't exist. In these cases, we just ignore it
// since such tests would not actually care.
const inlineStyleMap = getInlineStyleMap();
const filePath = inlineStyleMap.get(importPath);
const filePath = inlineStyleMap ? inlineStyleMap.get(importPath) : importPath;
if (!filePath) {
throw new Error(`Could not find "${
importPath}" in the inline style map. Available imports are:\n\n${
Array.from(inlineStyleMap.keys()).join('\n')}`);
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
Array.from(inlineStyleMap!.keys()).join('\n')}`);
}

// Return an annotation with the real file path.
Expand Down
10 changes: 10 additions & 0 deletions packages/rules_prerender/styles_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,16 @@ wksp/foo/bar/baz.css
wksp/hello/world.css
`.trim());
});

it('ignores the inline style map when not defined', () => {
spyOn(inlineStyleMap, 'getMap').and.returnValue(undefined);

expect(inlineStyle('wksp/foo/bar/baz.css')).toBe(`<!-- ${createAnnotation({
type: 'style',
scope: StyleScope.Inline,
path: 'wksp/foo/bar/baz.css',
})} -->`);
});
});

describe('inlineStyleLegacy()', () => {
Expand Down

0 comments on commit a720fff

Please sign in to comment.