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(dev-server): add "ping" route #5414

Merged
merged 6 commits into from
Mar 11, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
28 changes: 28 additions & 0 deletions src/compiler/config/test/validate-dev-server.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -276,4 +276,32 @@ describe('validateDevServer', () => {
const { config } = validateConfig(inputConfig, mockLoadConfigInit());
expect(config.devServer.prerenderConfig).toBe(wwwOutputTarget.prerenderConfig);
});

describe('pingRoute', () => {
it('should default to /ping', () => {
// Ensure the pingRoute is not set in the inputConfig so we know we're testing the
// default value added during validation
delete inputConfig.devServer.pingRoute;
const { config } = validateConfig(inputConfig, mockLoadConfigInit());
tanner-reits marked this conversation as resolved.
Show resolved Hide resolved
expect(config.devServer.pingRoute).toBe('/ping');
});

it('should set user defined pingRoute', () => {
inputConfig.devServer = { ...inputDevServerConfig, pingRoute: '/my-ping' };
const { config } = validateConfig(inputConfig, mockLoadConfigInit());
expect(config.devServer.pingRoute).toBe('/my-ping');
});

tanner-reits marked this conversation as resolved.
Show resolved Hide resolved
it('should prefix pingRoute with a "/"', () => {
inputConfig.devServer = { ...inputDevServerConfig, pingRoute: 'my-ping' };
const { config } = validateConfig(inputConfig, mockLoadConfigInit());
expect(config.devServer.pingRoute).toBe('/my-ping');
});

it('should clear ping route if set to null', () => {
inputConfig.devServer = { ...inputDevServerConfig, pingRoute: null };
const { config } = validateConfig(inputConfig, mockLoadConfigInit());
expect(config.devServer.pingRoute).toBe(null);
});
});
});
10 changes: 10 additions & 0 deletions src/compiler/config/validate-dev-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,16 @@ export const validateDevServer = (config: d.ValidatedConfig, diagnostics: d.Diag

devServer.address = devServer.address.split('/')[0];

// Validate "ping" route option
if (devServer.pingRoute !== null) {
let pingRoute = isString(devServer.pingRoute) ? devServer.pingRoute : '/ping';
if (!pingRoute.startsWith('/')) {
pingRoute = `/${pingRoute}`;
}

devServer.pingRoute = pingRoute;
}

// split on `:` to get the domain and the (possibly present) port
// separately. we've already sliced off the protocol (if present) above
// so we can safely split on `:` here.
Expand Down
9 changes: 9 additions & 0 deletions src/declarations/stencil-public-compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,15 @@ export interface DevServerConfig extends StencilDevServerConfig {
prerenderConfig?: string;
protocol?: 'http' | 'https';
srcIndexHtml?: string;

/**
* Route to be used for the "ping" sub-route of the Stencil dev server.
* This route will return a 200 status code once the Stencil build has finished.
* Setting this to `null` will disable the ping route.
*
* Defaults to `/ping`
*/
pingRoute?: string | null;
Copy link
Member Author

Choose a reason for hiding this comment

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

Went with pingRoute, but totally fine if we wanted to go with something like "health" or "status".

Copy link
Member

Choose a reason for hiding this comment

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

I'm good with pingRoute 👍

}

export interface HistoryApiFallback {
Expand Down
15 changes: 15 additions & 0 deletions src/dev-server/request-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,21 @@ export function createRequestHandler(devServerConfig: d.DevServerConfig, serverC
return serverCtx.serve302(req, res);
}

if (devServerConfig.pingRoute !== null && req.pathname === devServerConfig.pingRoute) {
return serverCtx
.getBuildResults()
.then((result) => {
if (!result.hasSuccessfulBuild) {
return serverCtx.serve500(incomingReq, res, 'Build not successful', 'build error');
}

res.writeHead(200, 'OK');
res.write('OK');
res.end();
})
.catch(() => serverCtx.serve500(incomingReq, res, 'Error getting build results', 'ping error'));
}

if (isDevClient(req.pathname) && devServerConfig.websocket) {
return serveDevClient(devServerConfig, serverCtx, req, res);
}
Expand Down
26 changes: 26 additions & 0 deletions src/dev-server/test/req-handler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,32 @@ describe('request-handler', () => {
expect(h).toBe(`88mph<iframe></iframe>`);
});
});

describe('pingRoute', () => {
it('should return a 200 for successful build', async () => {
serverCtx.getBuildResults = () =>
Promise.resolve({ hasSuccessfulBuild: true }) as Promise<d.CompilerBuildResults>;

const handler = createRequestHandler(devServerConfig, serverCtx);

req.url = '/ping';

await handler(req, res);
expect(res.$statusCode).toBe(200);
});

it('should return a 500 for unsuccessful build', async () => {
serverCtx.getBuildResults = () =>
Promise.resolve({ hasSuccessfulBuild: false }) as Promise<d.CompilerBuildResults>;

const handler = createRequestHandler(devServerConfig, serverCtx);

req.url = '/ping';

await handler(req, res);
expect(res.$statusCode).toBe(500);
});
});
});

interface TestServerResponse extends ServerResponse {
Expand Down
Loading