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

Consider serving and transforming all HTML files, rather than just limiting it to index files #110

Merged
merged 8 commits into from
Jan 21, 2024
Merged
Show file tree
Hide file tree
Changes from 7 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
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ Contributors: [@elturpin](https://github.com/elturpin), [@patreeceeo](https://gi

## 0.11.1 (2023-11-17)

- Mount middlewares that serve HTML at `config.root` instead of `/` ([#91](https://github.com/szymmis/vite-express/pull/91))
- Mount middlewares that serve HTML at `config.base` instead of `/` ([#91](https://github.com/szymmis/vite-express/pull/91))

Contributors: [@rmhaiderali](https://github.com/rmhaiderali)

Expand Down
60 changes: 28 additions & 32 deletions src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,6 @@ function info(msg: string) {
);
}

function isStaticFilePath(path: string) {
return path.match(/(\.\w+$)|@vite|@id|@react-refresh/);
}

async function getTransformedHTML(html: string, req: express.Request) {
return Config.transformer ? Config.transformer(html, req) : html;
}
Expand Down Expand Up @@ -108,9 +104,7 @@ async function resolveConfig(): Promise<ViteConfig> {
),
);
}
} catch (e) {
1;
}
} catch (e) {}

try {
const config = fs.readFileSync(getViteConfigPath(), "utf8");
Expand Down Expand Up @@ -176,7 +170,10 @@ async function injectStaticMiddleware(
middleware: RequestHandler,
) {
const config = await getViteConfig();
app.use(config.base, middleware);

app.use(config.base, (req, res, next) =>
req.path.endsWith(".html") ? next() : middleware(req, res, next),
);

const stubMiddlewareLayer = app._router.stack.find(
(layer: { handle?: RequestHandler }) => layer.handle === stubMiddleware,
Expand All @@ -198,20 +195,22 @@ function isIgnoredPath(path: string, req: express.Request) {
: Config.ignorePaths(path, req);
}

function findClosestIndexToRoot(
reqPath: string,
root: string,
): string | undefined {
function findFilePath(reqPath: string, root: string): string | undefined {
if (reqPath.endsWith(".html")) {
const pathToTest = path.join(root, reqPath);
if (fs.existsSync(pathToTest)) return pathToTest;
}

// find closest index to root
const basePath = reqPath.slice(0, reqPath.lastIndexOf("/"));
const dirs = basePath.split("/");

while (dirs.length > 0) {
const pathToTest = path.join(root, ...dirs, "index.html");
if (fs.existsSync(pathToTest)) {
return pathToTest;
}
if (fs.existsSync(pathToTest)) return pathToTest;
dirs.pop();
}

return undefined;
}

Expand All @@ -226,22 +225,19 @@ async function injectViteIndexMiddleware(

if (isIgnoredPath(req.path, req)) return next();

if (isStaticFilePath(req.path)) next();
else {
const indexPath = findClosestIndexToRoot(req.path, config.root);
if (indexPath === undefined) return next();

const template = fs.readFileSync(indexPath, "utf8");
let html = await server.transformIndexHtml(req.originalUrl, template);

try {
html = await getTransformedHTML(html, req);
res.send(html);
} catch (e) {
console.error(e);
res.status(500);
return next();
}
const indexPath = findFilePath(req.path, config.root);
if (indexPath === undefined) return next();

const template = fs.readFileSync(indexPath, "utf8");
Copy link
Owner

@szymmis szymmis Jan 17, 2024

Choose a reason for hiding this comment

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

I feel like we need to find more meaningful name instead of findFilePath for that function. I'm not sure how those HTML files should be called, templates maybe?
So it would be called findTemplateFilePath or even readTemplate and it would replace those couple of lines (obviously after adding this functionality to the function itself 😅)

Suggested change
const indexPath = findFilePath(req.path, config.root);
if (indexPath === undefined) return next();
const template = fs.readFileSync(indexPath, "utf8");
const template = readTemplate(req.path, config.root);
if (!template) return next();

What do you think about that?

let html = await server.transformIndexHtml(req.originalUrl, template);

try {
html = await getTransformedHTML(html, req);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

What if we include the precise file path of the html file as an input for the getTransformedHTML function? This could assist users in distinguishing between various files, particularly when working with multipage apps. Additionally, I'm uncertain about the optimal parameter ordering for the getTransformedHTML function.

Suggested change
const indexPath = findFilePath(req.path, config.root);
if (indexPath === undefined) return next();
const template = fs.readFileSync(indexPath, "utf8");
let html = await server.transformIndexHtml(req.originalUrl, template);
try {
html = await getTransformedHTML(html, req);
const templateFilePath = findTemplateFilePath(req.path, config.root);
if (templateFilePath === undefined) return next();
const template = fs.readFileSync(templateFilePath, "utf8");
let html = await server.transformIndexHtml(req.originalUrl, template);
try {
html = await getTransformedHTML(
html,
req,
templateFilePath.slice(config.root.length),
);

res.send(html);
} catch (e) {
console.error(e);
res.status(500);
return next();
}
});
}
Expand All @@ -253,7 +249,7 @@ async function injectIndexMiddleware(app: core.Express) {
app.use(config.base, async (req, res, next) => {
if (isIgnoredPath(req.path, req)) return next();

const indexPath = findClosestIndexToRoot(req.path, distPath);
const indexPath = findFilePath(req.path, distPath);
if (indexPath === undefined) return next();

let html = fs.readFileSync(indexPath, "utf8");
Expand Down
12 changes: 12 additions & 0 deletions tests/env/dist/main.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<!doctype html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<meta http-equiv="X-UA-Compatible" content="IE=edge" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Document</title>
</head>
<body>
<h1>main</h1>
</body>
</html>
12 changes: 12 additions & 0 deletions tests/env/main.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<!doctype html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<meta http-equiv="X-UA-Compatible" content="IE=edge" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Document</title>
</head>
<body>
<h1>main</h1>
</body>
</html>
8 changes: 8 additions & 0 deletions tests/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ test("Express app", async (done) => {
expect(response.text).toMatch(/<h1>index<\/h1>/);
response = await request(app).get("/route");
expect(response.text).toMatch(/<h1>index<\/h1>/);
response = await request(app).get("/index.html");
expect(response.text).toMatch(/<h1>index<\/h1>/);
response = await request(app).get("/main.html");
expect(response.text).toMatch(/<h1>main<\/h1>/);

it("html is served correctly");

Expand Down Expand Up @@ -247,6 +251,10 @@ test("Express app with transformer function", async (done) => {

it("html is served correctly");

expect(response.text).toMatch(/<meta name="test"\/>/);
response = await request(app).get("/index.html");
expect(response.text).toMatch(/<meta name="test"\/>/);
response = await request(app).get("/main.html");
expect(response.text).toMatch(/<meta name="test"\/>/);

it("html is transformed correctly");
Expand Down