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

fix: kill zombie builds if they complete #2318

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
fix: kill zombie builds if they complete
  • Loading branch information
deer committed Feb 15, 2024
commit 6e378e403739350f7ddc2cb9f3f0cdae1ee2dfe9
1 change: 1 addition & 0 deletions src/dev/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,5 @@ export async function build(
);

await Promise.all(plugins.map((plugin) => plugin.buildEnd?.()));
return "Build complete.";
}
5 changes: 4 additions & 1 deletion src/dev/dev_command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,10 @@ export async function dev(
state.config.dev = false;
state.loadSnapshot = false;
state.build = true;
await build(state);
const result = await build(state);
if (result === "Build complete.") {
Deno.exit();
}
} else if (config) {
const state = await getInternalFreshState(
manifest,
Expand Down
11 changes: 11 additions & 0 deletions tests/build_test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as path from "$std/path/mod.ts";
import {
$,
assert,
assertEquals,
assertNotMatch,
Expand Down Expand Up @@ -305,3 +306,13 @@ Deno.test("prefer static files from build dir", async () => {
},
);
});

Deno.test("zombie build ends", async (t) => {
const fixture = path.join(Deno.cwd(), "tests", "fixture_zombie_build");
const result = await $`deno run -A ${fixture}/dev.ts build`.captureCombined()
.timeout("30s").text();
assertStringIncludes(
result,
"The manifest has been generated for 1 routes and 1 islands.",
);
});
1 change: 1 addition & 0 deletions tests/deps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,4 @@ export * as JSONC from "https://deno.land/[email protected]/jsonc/mod.ts";
export * as colors from "https://deno.land/[email protected]/fmt/colors.ts";
export { STATUS_CODE } from "https://deno.land/[email protected]/http/status.ts";
export { stripAnsiCode } from "https://deno.land/[email protected]/fmt/colors.ts";
export { default as $ } from "https://deno.land/x/[email protected]/mod.ts";
39 changes: 39 additions & 0 deletions tests/fixture_zombie_build/deno.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
{
"lock": false,
"tasks": {
"check": "deno fmt --check && deno lint && deno check **/*.ts && deno check **/*.tsx",
"cli": "echo \"import '\\$fresh/src/dev/cli.ts'\" | deno run --unstable -A -",
"manifest": "deno task cli manifest $(pwd)",
"start": "deno run -A --watch=static/,routes/ dev.ts",
"build": "deno run -A dev.ts build",
"preview": "deno run -A main.ts",
"update": "deno run -A -r https://fresh.deno.dev/update ."
},
"lint": {
"rules": {
"tags": [
"fresh",
"recommended"
]
}
},
"exclude": [
"**/_fresh/*"
],
"imports": {
"$fresh/": "file:https:///Users/reed/code/denoland/fresh/",
"preact": "https://esm.sh/[email protected]",
"preact/": "https://esm.sh/[email protected]/",
"@preact/signals": "https://esm.sh/*@preact/[email protected]",
"@preact/signals-core": "https://esm.sh/*@preact/[email protected]",
"tailwindcss": "npm:[email protected]",
"tailwindcss/": "npm:/[email protected]/",
"tailwindcss/plugin": "npm:/[email protected]/plugin.js",
"$std/": "https://deno.land/[email protected]/"
},
"compilerOptions": {
"jsx": "react-jsx",
"jsxImportSource": "preact"
},
"nodeModulesDir": true
}
8 changes: 8 additions & 0 deletions tests/fixture_zombie_build/dev.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#!/usr/bin/env -S deno run -A --watch=static/,routes/

import dev from "$fresh/dev.ts";
import config from "./fresh.config.ts";

import "$std/dotenv/load.ts";

await dev(import.meta.url, "./main.ts", config);
6 changes: 6 additions & 0 deletions tests/fixture_zombie_build/fresh.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { defineConfig } from "$fresh/server.ts";
import tailwind from "$fresh/plugins/tailwind.ts";

export default defineConfig({
plugins: [tailwind()],
});
19 changes: 19 additions & 0 deletions tests/fixture_zombie_build/fresh.gen.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// DO NOT EDIT. This file is generated by Fresh.
// This file SHOULD be checked into source version control.
// This file is automatically updated during development when running `dev.ts`.

import * as $index from "./routes/index.tsx";
import * as $Network from "./islands/Network.tsx";
import { type Manifest } from "$fresh/server.ts";

const manifest = {
routes: {
"./routes/index.tsx": $index,
},
islands: {
"./islands/Network.tsx": $Network,
},
baseUrl: import.meta.url,
} satisfies Manifest;

export default manifest;
29 changes: 29 additions & 0 deletions tests/fixture_zombie_build/islands/Network.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { useEffect, useRef } from "preact/hooks";
import { Network } from "https://esm.sh/[email protected]/standalone";
import { IS_BROWSER } from "$fresh/runtime.ts";

export default function NetworkDiagram() {
const container = useRef<HTMLDivElement>(null);
const network = useRef<Network | null>(null);

const nodes = [{ id: 1, label: "Node 1" }, { id: 2, label: "Node 2" }];
const edges = [{ from: 1, to: 2 }];

useEffect(() => {
if (!container.current) return;
// this check isn't necessary because this doesn't run on the server regardless
// but this is the standard advice for not blocking the build, so i'll include it just to demonstrate
if (IS_BROWSER || !Deno.args.includes("build")) {
network.current = new Network(container.current, {
nodes: nodes,
edges: edges,
}, {});
}
}, []);

return (
<div class="m-4 p-4 border border-sky-500">
<div ref={container} />
</div>
);
}
13 changes: 13 additions & 0 deletions tests/fixture_zombie_build/main.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/// <reference no-default-lib="true" />
/// <reference lib="dom" />
/// <reference lib="dom.iterable" />
/// <reference lib="dom.asynciterable" />
/// <reference lib="deno.ns" />

import "$std/dotenv/load.ts";

import { start } from "$fresh/server.ts";
import manifest from "./fresh.gen.ts";
import config from "./fresh.config.ts";

await start(manifest, config);
13 changes: 13 additions & 0 deletions tests/fixture_zombie_build/routes/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { Head } from "$fresh/runtime.ts";
import Network from "../islands/Network.tsx";

export default function Home() {
return (
<>
<Head>
<link rel="stylesheet" href="styles.css" />
</Head>
<Network />
</>
);
}
3 changes: 3 additions & 0 deletions tests/fixture_zombie_build/static/styles.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
@tailwind base;
@tailwind components;
@tailwind utilities;
7 changes: 7 additions & 0 deletions tests/fixture_zombie_build/tailwind.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { type Config } from "tailwindcss";

export default {
content: [
"{routes,islands,components}/**/*.{ts,tsx}",
],
} satisfies Config;