fix: re-implement URL parsing with WHATWG URL API

[pmmmwh]

This migrates from using url.parse from native-url to actually just using the WHATWG URL API which handles both path-relative and protocol-relative URLs gracefully.

CC @nticaric

Fixes #323

[nticaric]

This will fix the issue but I think the code could be more clean. For example, I see there is a unit test which is covering the case where the port is 0. By definition, the port cannot be zero, so I think there is no need to cover such cases. It's like covering a case where the port is -23

[pmmmwh]

This will fix the issue but I think the code could be more clean.
For example, I see there is a unit test which is covering the case where the port is 0.

The truth is the port can be 0. By definition a 0 port means ANY available port, which is similar to the concept of an empty hostname (0.0.0.0 or [::]) and thus we need to gracefully handle it. It's a similar concept to using // protocol-relative URLs which signifies us to use whatever is possible according to the location.

[nticaric]

I'm not sure this is the right way to handle it. In my opinion it's better to throw an error then trying to default the port to 8080,
which also might be a wrong guess and the user won't know what's going on.

Do you have a practical example why someone would have port 0 in the first place?

[pmmmwh]

You can actually start devServer and tell it to use whatever random port is available by passing in 0 since that's the Linux default of a random port:

webpack/webpack-dev-server#1059

I would agree that is is very niche and very much an edge case. Since we're on the edge of a new release, I'm open to breaking changes. Would love to know if you have any more ideas on how we can improve 😄

[nticaric]

Here's how I think the script could be cleaned up. It will pass all the test and is more readable.

I would also throw an error if the port is 0, and hostname 0.0.0.0 or [::], but thats up to you

const getCurrentScriptSource = require('./getCurrentScriptSource');
const parseQuery = require('./parseQuery');

/**
 * @typedef {Object} SocketUrlParts
 * @property {string} [auth]
 * @property {string} [hostname]
 * @property {string} [protocol]
 * @property {string} [pathname]
 * @property {string} [port]
 */

/**
 * Parse current location and Webpack's `__resourceQuery` into parts that can create a valid socket URL.
 * @param {string} [resourceQuery] The Webpack `__resourceQuery` string.
 * @returns {SocketUrlParts} The parsed URL parts.
 * @see https://webpack.js.org/api/module-variables/#__resourcequery-webpack-specific
 */
function getSocketUrlParts(resourceQuery) {
  const scriptSource = getCurrentScriptSource();

  let url = new URL(scriptSource, window.location);
  let auth;
  let hostname = url.hostname === '[::]' || url.hostname === '0.0.0.0' ? "localhost" : url.hostname;
  let protocol =
    url.protocol === 'https:' || window.location.protocol === 'https:' ? 'https:' : 'http:';
  let pathname = '/sockjs-node'; // This is hard-coded in WDS
  let port = url.port === '0' || url.port === '' ? '8080' : url.port;

  if (url.username) {
    auth = [url.username, url.password].join(':').replace(/:$/g, '');
  }

  // If the resource query is available,
  // parse it and overwrite everything we received from the script host.
  const parsedQuery = parseQuery(resourceQuery || '');
  hostname = parsedQuery.sockHost || hostname;
  pathname = parsedQuery.sockPath || pathname;
  port = parsedQuery.sockPort || port;

  // Make sure the protocol from resource query has a trailing colon
  if (parsedQuery.sockProtocol) {
    protocol = parsedQuery.sockProtocol + ':';
  }

  if (!scriptSource && !resourceQuery) {
    throw new Error(
      [
        '[React Refresh] Failed to get an URL for the socket connection.',
        "This usually means that the current executed script doesn't have a `src` attribute set.",
        'You should either specify the socket path parameters under the `devServer` key in your Webpack config, or use the `overlay` option.',
        'https://github.com/pmmmwh/react-refresh-webpack-plugin/blob/main/docs/API.md#overlay',
      ].join('\n'))
  }

  return {
    auth: auth,
    hostname: hostname,
    pathname: pathname,
    protocol: protocol,
    port: port,
  };
}

module.exports = getSocketUrlParts;

[pmmmwh]

Let's merge this before we cleanup 😄 .

I think at least removing the 0 port would make sense, the host names I'm not quite sure because they can be quite useful under proxies, and/or used as implicit references to localhost