Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 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
15 changes: 8 additions & 7 deletions packages/fresh/src/commands.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { setAdditionalStyles } from "./context.ts";
import { HttpError } from "./error.ts";
import { isHandlerByMethod, type PageResponse } from "./handlers.ts";
import {
Expand Down Expand Up @@ -74,31 +73,36 @@ export function newErrorCmd<State>(
export interface AppCommand<State> {
type: CommandType.App;
component: RouteComponent<State>;
css?: string[];
}
export function newAppCmd<State>(
component: RouteComponent<State>,
css?: string[],
): AppCommand<State> {
return { type: CommandType.App, component };
return { type: CommandType.App, component, css };
}

export interface LayoutCommand<State> {
type: CommandType.Layout;
pattern: string;
component: RouteComponent<State>;
config?: LayoutConfig;
css?: string[];
includeLastSegment: boolean;
}
export function newLayoutCmd<State>(
pattern: string,
component: RouteComponent<State>,
config: LayoutConfig | undefined,
includeLastSegment: boolean,
css?: string[],
): LayoutCommand<State> {
return {
type: CommandType.Layout,
pattern,
component,
config,
css,
includeLastSegment,
};
}
Expand Down Expand Up @@ -253,7 +257,7 @@ function applyCommandsInner<State>(
break;
}
case CommandType.App: {
root.app = cmd.component;
root.app = { component: cmd.component, css: cmd.css ?? null };
break;
}
case CommandType.Layout: {
Expand All @@ -265,6 +269,7 @@ function applyCommandsInner<State>(
segment.layout = {
component: cmd.component,
config: cmd.config ?? null,
css: cmd.css ?? null,
};
break;
}
Expand All @@ -290,10 +295,6 @@ function applyCommandsInner<State>(
def = await route();
}

if (def.css !== undefined) {
setAdditionalStyles(ctx, def.css);
}

return renderRoute(ctx, def);
});

Expand Down
41 changes: 33 additions & 8 deletions packages/fresh/src/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,11 @@ export type ServerIslandRegistry = Map<ComponentType, Island>;
export const internals: unique symbol = Symbol("fresh_internal");

export interface UiTree<Data, State> {
app: AnyComponent<PageProps<Data, State>> | null;
layouts: ComponentDef<Data, State>[];
app: {
component: AnyComponent<PageProps<Data, State>>;
css: string[] | null;
} | null;
layouts: (ComponentDef<Data, State> & { css: string[] | null })[];
}

/**
Expand All @@ -109,7 +112,10 @@ export type FreshContext<State = unknown> = Context<State>;

export let getBuildCache: <T>(ctx: Context<T>) => BuildCache<T>;
export let getInternals: <T>(ctx: Context<T>) => UiTree<unknown, T>;
export let setAdditionalStyles: <T>(ctx: Context<T>, css: string[]) => void;
export let setAdditionalStyles: <T>(
ctx: Context<T>,
css: string[] | null | undefined,
) => void;

/**
* The context passed to every middleware. It is unique for every request.
Expand Down Expand Up @@ -182,8 +188,24 @@ export class Context<State> {
// deno-lint-ignore no-explicit-any
getInternals = <T>(ctx: Context<T>) => ctx.#internal as any;
getBuildCache = <T>(ctx: Context<T>) => ctx.#buildCache;
setAdditionalStyles = <T>(ctx: Context<T>, css: string[]) =>
ctx.#additionalStyles = css;
setAdditionalStyles = <T>(
ctx: Context<T>,
css: string[] | null | undefined,
) => {
if (css == null) return;

if (ctx.#additionalStyles === null) {
ctx.#additionalStyles = css.slice();
return;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor: includes() is O(n) per insertion. For the small CSS lists in practice this is fine, but a Set would be more idiomatic if you ever want to tighten this up. Low priority.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yep, I added a FIXME comment below. I think it could be use Set instead of css: Array<string>.It seems that at least all the tests pass in the following branch.

Hajime-san/fresh@fix-ssr-css-modules...Hajime-san:fresh:perf-css

I'm planning it as a follow up PR.

}

for (let i = 0; i < css.length; i++) {
const href = css[i];
if (!ctx.#additionalStyles.includes(href)) {
ctx.#additionalStyles.push(href);
}
}
};
}

constructor(
Expand Down Expand Up @@ -283,6 +305,7 @@ export class Context<State> {
props.Component = () => child;

const def = defs[i];
setAdditionalStyles(this, def.css);

const result = await renderRouteComponent(this, def, () => child);
if (result instanceof Response) {
Expand All @@ -298,16 +321,18 @@ export class Context<State> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: appDef?.css evaluates to undefined when appDef is null, which the new setAdditionalStyles handles via the css == null guard. Works correctly, but it's a subtle null-propagation chain. An explicit guard might be clearer:

if (appDef !== null) {
  setAdditionalStyles(this, appDef.css);
}

Especially since the appDef !== null checks already exist a few lines below.

let hasApp = true;

if (isAsyncAnyComponent(appDef)) {
setAdditionalStyles(this, appDef?.css);

if (appDef !== null && isAsyncAnyComponent(appDef.component)) {
props.Component = () => appChild;
const result = await renderAsyncAnyComponent(appDef, props);
const result = await renderAsyncAnyComponent(appDef.component, props);
if (result instanceof Response) {
return result;
}

appVNode = result;
} else if (appDef !== null) {
appVNode = h(appDef, {
appVNode = h(appDef.component, {
Component: () => appChild,
config: this.config,
data: null,
Expand Down
8 changes: 6 additions & 2 deletions packages/fresh/src/fs_routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,9 @@ export function fsItemsToCommands<State>(
}
if (!mod.default) continue;

commands.push(newLayoutCmd(pattern, mod.default, mod.config, true));
commands.push(
newLayoutCmd(pattern, mod.default, mod.config, true, mod.css),
);
continue;
}
case CommandType.Error: {
Expand All @@ -116,6 +118,7 @@ export function fsItemsToCommands<State>(
{
component: mod.default ?? undefined,
config: mod.config ?? undefined,
css: mod.css,
// deno-lint-ignore no-explicit-any
handler: (handlers as any) ?? undefined,
},
Expand All @@ -128,6 +131,7 @@ export function fsItemsToCommands<State>(
commands.push(newNotFoundCmd({
config: mod.config,
component: mod.default,
css: mod.css,
// deno-lint-ignore no-explicit-any
handler: handlers as any ?? undefined,
}));
Expand All @@ -137,7 +141,7 @@ export function fsItemsToCommands<State>(
const { mod } = validateFsMod<State>(filePath, rawMod, type);
if (mod.default === undefined) continue;

commands.push(newAppCmd(mod.default));
commands.push(newAppCmd(mod.default, mod.css));
continue;
}
case CommandType.Route: {
Expand Down
16 changes: 13 additions & 3 deletions packages/fresh/src/segments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { AnyComponent } from "preact";
import type { MaybeLazyMiddleware, Middleware } from "./middlewares/mod.ts";
import { type Method, patternToSegments } from "./router.ts";
import type { LayoutConfig, Route } from "./types.ts";
import { type Context, getInternals } from "./context.ts";
import { type Context, getInternals, setAdditionalStyles } from "./context.ts";
import { recordSpanError, tracer } from "./otel.ts";
import { type HandlerFn, isHandlerByMethod } from "./handlers.ts";
import {
Expand All @@ -22,10 +22,14 @@ export interface Segment<State> {
layout: {
component: RouteComponent<State>;
config: LayoutConfig | null;
css: string[] | null;
} | null;
errorRoute: Route<State> | null;
notFound: Middleware<State> | null;
app: RouteComponent<State> | null;
app: {
component: RouteComponent<State>;
css: string[] | null;
} | null;
children: Map<string, Segment<State>>;
parent: Segment<State> | null;
}
Expand Down Expand Up @@ -105,7 +109,11 @@ export function segmentToMiddlewares<State>(
internals.app = null;
}

const def = { props: null, component: layout.component };
const def = {
props: null,
component: layout.component,
css: layout.css,
};
if (layout.config?.skipInheritedLayouts) {
internals.layouts = [def];
} else {
Expand Down Expand Up @@ -145,6 +153,8 @@ export async function renderRoute<State>(
route: Route<State>,
status = 200,
): Promise<Response> {
setAdditionalStyles(ctx, route.css);

const internals = getInternals(ctx);
if (route.config?.skipAppWrapper) {
internals.app = null;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { CssModulesNonIsland } from "../../../components/CssModuleNonIsland.tsx";
import { define } from "../../../utils.ts";

export default define.layout(({ Component }) => {
return (
<>
<CssModulesNonIsland />
<Component />
</>
);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <p>non-island CSS Modules</p>;
}
16 changes: 16 additions & 0 deletions packages/plugin-vite/src/plugins/dev_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ export function devServer(freshConfig: ResolvedFreshViteConfig): Plugin[] {
res.headers.get("Content-Type")?.includes("text/html")
) {
const clientEnv = server.environments.client;
const ssrEnv = server.environments.ssr;
const collected = await collectCss(
"fresh:client-entry",
clientEnv,
Expand All @@ -145,6 +146,21 @@ export function devServer(freshConfig: ResolvedFreshViteConfig): Plugin[] {
}
}

// Route/app/layout/error CSS lives behind fresh-route-css virtual
// modules that are first discovered in the SSR graph.
for (const mod of ssrEnv.moduleGraph.idToModuleMap.values()) {
if (mod.id?.includes("fresh-route-css::")) {
let id = mod.id;
if (id.startsWith("\0fresh-route-css::")) {
id = `/@id/fresh-route-css::${
id.slice("\0fresh-route-css::".length)
}.module.css`;
}
const routeCss = await collectCss(id, clientEnv);
collected.push(...routeCss);
}
}

let html = await res.text();

const styles = collected.join("\n");
Expand Down
Loading
Loading