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.

Sign up for GitHub

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

automatically configure wsPort in html file #1147

Merged
merged 1 commit into from
Oct 14, 2020
Merged

automatically configure wsPort in html file #1147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 6 additions & 3 deletions snowpack/assets/hmr-client.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,12 @@ function sendSocketMessage(msg) {
_sendSocketMessage(msg);
}
}
const socketURL =
(typeof window !== 'undefined' && window.HMR_WEBSOCKET_URL) ||
(location.protocol === 'http:' ? 'ws://' : 'wss://') + location.host + '/';
let socketURL = typeof window !== 'undefined' && window.HMR_WEBSOCKET_URL;
if (!socketURL) {
const socketHost = typeof window !== 'undefined' && window.HMR_WEBSOCKET_PORT ?
`${location.hostname}:${window.HMR_WEBSOCKET_PORT}` : location.host
socketURL = (location.protocol === 'http:' ? 'ws://' : 'wss://') + socketHost + '/';
}

const socket = new WebSocket(socketURL, 'esm-hmr');
socket.addEventListener('open', () => {
Expand Down
8 changes: 7 additions & 1 deletion snowpack/src/build/build-import-proxy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,14 @@ export function wrapImportMeta({
export function wrapHtmlResponse({
code,
hmr,
hmrPort,
isDev,
config,
mode,
}: {
code: string;
hmr: boolean;
hmrPort?: number;
isDev: boolean;
config: SnowpackConfig;
mode: 'development' | 'production';
Expand Down Expand Up @@ -88,7 +90,11 @@ export function wrapHtmlResponse({
// Any code not containing `<!DOCTYPE html>` is assumed to be a code snippet/partial.
const isFullPage = code.startsWith('<!DOCTYPE html>');
if (hmr && isFullPage) {
let hmrScript = `<script type="module" integrity="${SRI_CLIENT_HMR_SNOWPACK}" src="${getMetaUrlPath(
let hmrScript = ``;
if (hmrPort) {
hmrScript += `<script type="text/javascript">window.HMR_WEBSOCKET_PORT=${hmrPort}</script>\n`;
}
hmrScript += `<script type="module" integrity="${SRI_CLIENT_HMR_SNOWPACK}" src="${getMetaUrlPath(
'hmr-client.js',
config,
)}"></script>`;
Expand Down
3 changes: 2 additions & 1 deletion snowpack/src/commands/build.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ class FileBuilder {
code = wrapHtmlResponse({
code,
hmr: getIsHmrEnabled(this.config),
hmrPort: hmrEngine ? hmrEngine.port : undefined,
isDev: false,
config: this.config,
mode: 'production',
Expand Down Expand Up @@ -474,7 +475,7 @@ export async function command(commandOptions: CommandOptions) {
if (hmrEngine) {
logger.info(
`[HMR] WebSocket URL available at ${colors.cyan(
`ws://localhost:${config.devOptions.hmrPort}`,
`ws://localhost:${hmrEngine.port}`,
)}`,
);
}
Expand Down
7 changes: 6 additions & 1 deletion snowpack/src/commands/dev.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,7 @@ export async function startServer(commandOptions: CommandOptions) {
code = wrapHtmlResponse({
code: code as string,
hmr: isHmr,
hmrPort: hmrEngine.port !== port ? hmrEngine.port : undefined,
isDev: true,
config,
mode: 'development',
Expand Down Expand Up @@ -989,7 +990,11 @@ export async function startServer(commandOptions: CommandOptions) {
.listen(port);

const {hmrDelay} = config.devOptions;
const hmrEngine = new EsmHmrEngine({server, delay: hmrDelay});
const hmrEngineOptions = Object.assign(
{delay: hmrDelay},
config.devOptions.hmrPort ? {port: config.devOptions.hmrPort} : {server, port},
);
const hmrEngine = new EsmHmrEngine(hmrEngineOptions);
onProcessExit(() => {
hmrEngine.disconnectAllClients();
});
Expand Down
2 changes: 1 addition & 1 deletion snowpack/src/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ const DEFAULT_CONFIG: Partial<SnowpackConfig> = {
output: 'dashboard',
fallback: 'index.html',
hmrDelay: 0,
hmrPort: 12321,
Copy link
Owner

Choose a reason for hiding this comment

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

Any reason to remove this? Whenever possible, it's good to give these defaults so that the rest of the codebase doesn't need to handle undefined values. So I'd vote to keep this here and remove DEFAULT_PORT

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we set default hmrPort to 12321, we cannot distinguish between snowpack dev and snowpack dev --hmrPort=12321. Because we use port of devServer as hmrEngine's port while using snowpack dev, but we use 12321 as hmrEngine's port while using snowpack dev --hmrPort=12321.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@FredKSchott There is another way to implement it. we can add a DEFAULT_BUILD_CONFIG in this file and merge DEFAULT_BUILD_CONFIG to config if snowpack is in build mode. The code of DEFAULT_CONFIG and DEFAULT_BUILD_CONFIG is as below.

const DEFAULT_CONFIG = {
  // other default config
  devOptions: {
     hmrPort: undefined,
  }
}

const DEFAULT_BUILD_CONFIG = {
  devOptions: {
     hmrPort: 12321,
  }
}

I prefer this way since we usually need to set different configuration according to whether snowpack runs in dev mode or build mode.

I would like to use this way to modify this pr if you agree with it 😁.

Copy link
Owner

Choose a reason for hiding this comment

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

Ah right, of course. Okay, let's keep as it is now for this PR. If you'd like to create a follow up PR to simplify this config further, that would be great!

hmrPort: undefined,
hmrErrorOverlay: true,
},
buildOptions: {
Expand Down
9 changes: 6 additions & 3 deletions snowpack/src/hmr-server-engine.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ type HMRMessage =
};

const DEFAULT_CONNECT_DELAY = 2000;
const DEFAULT_PORT = 12321;

interface EsmHmrEngineOptionsCommon {
delay?: number;
Expand All @@ -32,10 +33,10 @@ interface EsmHmrEngineOptionsCommon {
type EsmHmrEngineOptions = (
| {
server: http.Server | http2.Http2Server;
port?: undefined;
port: number;
}
| {
port: number;
port?: number;
server?: undefined;
}
) &
Expand All @@ -49,11 +50,13 @@ export class EsmHmrEngine {
private currentBatch: HMRMessage[] = [];
private currentBatchTimeout: NodeJS.Timer | null = null;
private cachedConnectErrors: Set<HMRMessage> = new Set();
readonly port: number = 0;

constructor(options: EsmHmrEngineOptions) {
this.port = options.port || DEFAULT_PORT;
const wss = options.server
? new WebSocket.Server({noServer: true})
: new WebSocket.Server({port: options.port});
: new WebSocket.Server({port: this.port});
if (options.delay) {
this.delay = options.delay;
}
Expand Down
2 changes: 1 addition & 1 deletion snowpack/src/types/snowpack.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ export interface SnowpackConfig {
output: 'stream' | 'dashboard';
hmr?: boolean;
hmrDelay: number;
hmrPort: number;
hmrPort: number | undefined;
hmrErrorOverlay: boolean;
};
installOptions: InstallOptions;
Expand Down
2 changes: 1 addition & 1 deletion test-dev/__snapshots__/dev.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ exports[`snowpack dev smoke: html 1`] = `
<meta name=\\"description\\" content=\\"Web site created using create-snowpack-app\\" />
<link rel=\\"stylesheet\\" type=\\"text/css\\" href=\\"/index.css\\" />
<title>Snowpack App</title>
<script type=\\"module\\" integrity=\\"sha384-SyNG1azQc4Pnqnk9u4TrKAXskp47aq9Zj8HDof3MZ57tq1p+rSl5recyqsc08CcH\\" src=\\"/__snowpack__/hmr-client.js\\"></script><script type=\\"module\\" integrity=\\"sha384-ZHQS30EqaFTVzyH5XP0i+sVAiN0+DiOd1AikxE6LsJbQaA40xjdaXluCLs/7hPlM\\" src=\\"/__snowpack__/hmr-error-overlay.js\\"></script></head>
<script type=\\"module\\" integrity=\\"sha384-Z1OPSs1KV1TOmQHeoGF8XyUQcMaBnM0p0xip/dmSSpZWySqHrGOuQ5R4sSJgwPrM\\" src=\\"/__snowpack__/hmr-client.js\\"></script><script type=\\"module\\" integrity=\\"sha384-ZHQS30EqaFTVzyH5XP0i+sVAiN0+DiOd1AikxE6LsJbQaA40xjdaXluCLs/7hPlM\\" src=\\"/__snowpack__/hmr-error-overlay.js\\"></script></head>
<body>
<img id=\\"img\\" src=\\"/logo.svg\\" />
<canvas id=\\"canvas\\"></canvas>
Expand Down