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

feat(server): add safety checks during dev #1568

Open
wants to merge 33 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
82c3d2f
safety checks prototypes
deer Jul 13, 2023
eb122a0
chore: add remainig safety checks
mbhrznr Jul 19, 2023
0b481d3
chore: add remainig safety checks
mbhrznr Jul 19, 2023
d71919c
docs(island): remove section about island naming
mbhrznr Jul 19, 2023
b56f643
fix(tests): resolve async ops
mbhrznr Jul 28, 2023
6bc5202
feat(server): add check for route conflicts
mbhrznr Jul 28, 2023
bc384b0
Merge branch 'main' into feat/safety_checks
mbhrznr Aug 1, 2023
07d19c6
chore: add @ts-expect-error to fixture
mbhrznr Aug 1, 2023
9c3f128
chore: add handler function check
mbhrznr Aug 1, 2023
47d8fa9
chore: add tests for dev_checks
mbhrznr Aug 3, 2023
5963bd5
Merge branch 'main' into feat/safety_checks
mbhrznr Aug 3, 2023
93e4f53
chore: adapt tests to adhere to new route type
mbhrznr Aug 3, 2023
482c43d
chore: solidify checks and tests
mbhrznr Aug 7, 2023
4975673
Merge branch 'main' into feat/safety_checks
mbhrznr Aug 7, 2023
7699aae
chore: update test to contain multiple methods
mbhrznr Aug 7, 2023
d2515e1
chore: add some docs to dev_checks
mbhrznr Aug 7, 2023
e79b4ae
chore: revert type declaration to 'as Plugin'
mbhrznr Aug 8, 2023
5b6a2e9
chore: improve warnings output and update tests
mbhrznr Aug 9, 2023
8a58cff
Merge branch 'main' into feat/safety_checks
mbhrznr Aug 9, 2023
c110179
chore: fix typo in check category
mbhrznr Aug 9, 2023
32c835c
chore: implement review remarks
mbhrznr Aug 9, 2023
2d2cacb
Merge branch 'main' into feat/safety_checks
mbhrznr Aug 10, 2023
75d3ee0
chore: update tests after merging main
mbhrznr Aug 10, 2023
f8675bf
Merge branch 'main' into feat/safety_checks
mbhrznr Aug 21, 2023
02c418f
chore: update tests after merging main
mbhrznr Aug 21, 2023
5a6bc69
chore: add check for dynamic route params
mbhrznr Aug 25, 2023
062ca65
chore: fix windows tests
mbhrznr Aug 25, 2023
78c2b77
Merge branch 'main' into feat/safety_checks
mbhrznr Sep 4, 2023
f83638a
chore: update logic and tests for conflicting dynamic routes
mbhrznr Sep 4, 2023
584994c
chore: minor tweaks
mbhrznr Sep 4, 2023
0213064
chore: update tests
mbhrznr Sep 4, 2023
31ad27c
chore: add plugin dev check for aot builds
mbhrznr Sep 4, 2023
f6d9721
Merge branch 'main' into feat/safety_checks
mbhrznr Sep 12, 2023
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
3 changes: 1 addition & 2 deletions docs/latest/concepts/islands.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ is different from all other components in Fresh, as they are usually rendered on
the server only.

Islands are defined by creating a file in the `islands/` folder in a Fresh
project. The name of this file must be a PascalCase or kebab-case name of the
island.
project.

```tsx islands/my-island.tsx
import { useSignal } from "@preact/signals";
Expand Down
7 changes: 7 additions & 0 deletions docs/latest/getting-started/dynamic-routes.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@ render a page. The module must once again expose a component as a default
export. This time the component will receive the matched path segment properties
as arguments in its `props` object though.

> :warning: Warning: it is possible to create both static and dynamic routes in
> the same directory, possibly leading to confusion. Fresh will always choose
> the static route first. E.g. `/nested/index.ts` and `/nested/static.ts` will
> always route ahead of `/nested/[id].ts`. Fresh will warn you of multiple
> conflicting dynamic routes in development mode, e.g. `/nested/[dynamic1].ts`
> and `/nested/[dynamic2].ts`.

```tsx routes/greet/[name].tsx
import { PageProps } from "$fresh/server.ts";

Expand Down
87 changes: 69 additions & 18 deletions src/server/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import {
RouterOptions,
RouterState,
ServeHandlerInfo,
StaticFile,
UnknownPage,
UnknownPageModule,
} from "./types.ts";
Expand All @@ -57,6 +58,22 @@ import {
JSXConfig,
} from "../build/mod.ts";
import { InternalRoute } from "./router.ts";
import {
assertModuleExportsDefault,
assertNoDynamicRouteConflicts,
assertNoStaticRouteConflicts,
assertPluginsCallRender,
assertPluginsCallRenderAsync,
assertPluginsDuringAOTBuild,
assertPluginsInjectModules,
assertRoutesHaveHandlerOrComponent,
assertSingleModule,
assertSingleRoutePattern,
assertStaticDirSafety,
CheckCategory,
CheckFunction,
CheckResult,
} from "./dev_checks.ts";
import { setAllIslands } from "./rendering/preact_hooks.ts";

const DEFAULT_CONN_INFO: ServeHandlerInfo = {
Expand All @@ -76,20 +93,6 @@ function isDevMode() {
// Env var is only set in prod (on Deploy).
return Deno.env.get("DENO_DEPLOYMENT_ID") === undefined;
}

interface StaticFile {
/** The URL to the static file on disk. */
localUrl: URL;
/** The path to the file as it would be in the incoming request. */
path: string;
/** The size of the file. */
size: number;
/** The content-type of the file. */
contentType: string;
/** Hash of the file contents. */
etag: string;
}

export class ServerContext {
#dev: boolean;
#routes: Route[];
Expand Down Expand Up @@ -152,6 +155,7 @@ export class ServerContext {
): Promise<ServerContext> {
// Get the manifest' base URL.
const baseUrl = new URL("./", manifest.baseUrl).href;
const defaultStaticDir = "./static";

const { config, path: configPath } = await readDenoConfig(
fromFileUrl(baseUrl),
Expand All @@ -164,6 +168,7 @@ export class ServerContext {

// Restore snapshot if available
let snapshot: BuildSnapshot | null = null;
let json: BuildSnapshotJson | null = null;
// Load from snapshot if not explicitly requested not to
const loadFromSnapshot = !opts.skipSnapshot;
if (loadFromSnapshot) {
Expand All @@ -175,9 +180,11 @@ export class ServerContext {
);

const snapshotPath = join(snapshotDirPath, "snapshot.json");
const json = JSON.parse(

json = JSON.parse(
await Deno.readTextFile(snapshotPath),
) as BuildSnapshotJson;

setBuildId(json.build_id);

const dependencies = new Map<string, string[]>(
Expand Down Expand Up @@ -224,13 +231,16 @@ export class ServerContext {
const islands: Island[] = [];
const middlewares: MiddlewareRoute[] = [];
let app: AppModule = DEFAULT_APP;
let appModule: { url: string; module: AppModule } | null = null;
const layouts: LayoutRoute[] = [];
let notFound: UnknownPage = DEFAULT_NOT_FOUND;
let unknownModule: { url: string; module: UnknownPageModule } | null = null;
let error: ErrorPage = DEFAULT_ERROR;
let errorModule: { url: string; module: ErrorPageModule } | null = null;
const allRoutes = [
...Object.entries(manifest.routes),
...(opts.plugins ? getMiddlewareRoutesFromPlugins(opts.plugins) : []),
...(opts.plugins ? getRoutesFromPlugins(opts.plugins) : []),
...getMiddlewareRoutesFromPlugins(opts.plugins ?? []),
...getRoutesFromPlugins(opts.plugins ?? []),
];

// Presort all routes so that we only need to sort once
Expand Down Expand Up @@ -309,6 +319,7 @@ export class ServerContext {
path === "/_app.jsx" || path === "/_app.js"
) {
app = module as AppModule;
appModule = { url, module: module as AppModule };
} else if (isLayout) {
const mod = module as LayoutModule;
const config = mod.config;
Expand All @@ -323,6 +334,7 @@ export class ServerContext {
path === "/_404.tsx" || path === "/_404.ts" ||
path === "/_404.jsx" || path === "/_404.js"
) {
unknownModule = { url, module: module as UnknownPageModule };
const { default: component, config } = module as UnknownPageModule;
let { handler } = module as UnknownPageModule;
if (component && handler === undefined) {
Expand All @@ -344,6 +356,7 @@ export class ServerContext {
path === "/_500.tsx" || path === "/_500.ts" ||
path === "/_500.jsx" || path === "/_500.js"
) {
errorModule = { url, module: module as ErrorPageModule };
const { default: component, config } = module as ErrorPageModule;
let { handler } = module as ErrorPageModule;
if (component && handler === undefined) {
Expand Down Expand Up @@ -395,7 +408,7 @@ export class ServerContext {
const staticFiles: StaticFile[] = [];
try {
const staticFolder = new URL(
opts.staticDir ?? "./static",
opts.staticDir ?? defaultStaticDir,
manifest.baseUrl,
);
const entries = walk(fromFileUrl(staticFolder), {
Expand Down Expand Up @@ -437,6 +450,44 @@ export class ServerContext {

const dev = opts.dev ?? isDevMode();
if (dev) {
const checks: CheckFunction[] = [
() => assertModuleExportsDefault(appModule, "_app"),
() => assertSingleModule(routes, "_app"),
() => assertModuleExportsDefault(unknownModule, "_404"),
() => assertSingleModule(routes, "_404"),
() => assertSingleRoutePattern(routes),
() => assertModuleExportsDefault(errorModule, "_500"),
() => assertSingleModule(routes, "_500"),
() => assertRoutesHaveHandlerOrComponent(routes),
() => assertNoDynamicRouteConflicts(routes),
() => assertNoStaticRouteConflicts(routes, staticFiles),
() => assertStaticDirSafety(opts.staticDir ?? "", defaultStaticDir),
() => assertPluginsCallRender(opts.plugins ?? []),
() => assertPluginsCallRenderAsync(opts.plugins ?? []),
() => assertPluginsInjectModules(opts.plugins ?? []),
() => assertPluginsDuringAOTBuild(opts.plugins ?? [], json),
];

const results = checks.flatMap((check) => check());
const resultsByCategory = new Map<CheckCategory, CheckResult[]>(
Object.values(CheckCategory).map((category) => [category, []]),
);

for (const result of results) {
resultsByCategory.get(result.category)?.push(result);
}

for (const [category, results] of resultsByCategory) {
for (const result of results) {
console.log(`%c${category}`, "font-weight:bold");
console.log(` ${result.message}`);
if (result.link) {
const link = result.link.substring(baseUrl.length);
console.log(` See: ${link ? `./${link}` : result.link}`);
}
}
}

// Ensure that debugging hooks are set up for SSR rendering
await import("preact/debug");
}
Expand Down
5 changes: 5 additions & 0 deletions src/server/deps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export {
} from "https://deno.land/[email protected]/path/mod.ts";
export { walk } from "https://deno.land/[email protected]/fs/walk.ts";
export * as colors from "https://deno.land/[email protected]/fmt/colors.ts";
export { existsSync } from "https://deno.land/[email protected]/fs/mod.ts";
export {
type Handler as ServeHandler,
serve,
Expand All @@ -19,3 +20,7 @@ export {
export { toHashString } from "https://deno.land/[email protected]/crypto/to_hash_string.ts";
export { escape } from "https://deno.land/[email protected]/regexp/escape.ts";
export * as JSONC from "https://deno.land/[email protected]/jsonc/mod.ts";
export {
assertSpyCalls,
spy,
} from "https://deno.land/[email protected]/testing/mock.ts";
Loading