Repository: pmmmwh/react-refresh-webpack-plugin (js)

Issue:
Fast refresh error - can't access property "trim", t is null
## Issue

In [this example repo](https://github.com/frehner/react-fast-refresh-externals), I get the following error 100% of the time:

`Uncaught (in promise) TypeError: can't access property "trim", t is null`

in debugging, it comes from this function 

https://github.com/pmmmwh/react-refresh-webpack-plugin/blob/89ccca3a528a68f06f777daef09a28a5614e5525/sockets/utils/getCurrentScriptSource.js#L5

which does various things to try and figure out the current script source. However, in that example repo, `document.currentScript` is null AND `currentScript.getAttribute('src');` is null - which then causes issues down the line: specifically here (where the error is eventually thrown)

https://github.com/pmmmwh/react-refresh-webpack-plugin/blob/89ccca3a528a68f06f777daef09a28a5614e5525/sockets/utils/getSocketUrlParts.js#L22

## Fix proposal

With that information, I propose the following change to the `getCurrentScriptSource` (with the caveat that I am not a webpack plugin developer, so I don't know the ins-and-outs of webpack plugins and this may not be the correct solution. However, I have tested this proposal and it appears to fix everything in that example repo)

```js
getCurrentScriptSource() {
  return __webpack_public_path__
}
```

which, again, fixes the issue for that example repo... and potentially fixes any issues for anyone using a [dynamic public path](https://webpack.js.org/guides/public-path/#on-the-fly) as well?

If you think that's the correct way to go, then I can make a PR for it. 

Thanks for this project, and I'm looking forward to your thoughts.

Pull request:
fix: adds resourceQuery to getSocketUrlParts to setup the socket
Fixes #216 

by default it pulls from WDS's devServer option in the webpack config, and passes that as a resourceQuery.

getSocketUrlParts changes allow it to use the resourceQuery primarily, and then fall back to the old method if resourceQuery isn't there.

Patch:
diff --git a/lib/index.js b/lib/index.js
index 378307d..2bdf5d6 100644
--- a/lib/index.js
+++ b/lib/index.js
@@ -10,6 +10,7 @@ const {
   injectRefreshEntry,
   injectRefreshLoader,
   normalizeOptions,
+  getAdditionalEntries,
 } = require('./utils');
 const schema = require('./options.json');
 
@@ -85,7 +86,10 @@ class ReactRefreshPlugin {
     }
 
     // Inject react-refresh context to all Webpack entry points
-    compiler.options.entry = injectRefreshEntry(compiler.options.entry, this.options);
+    compiler.options.entry = injectRefreshEntry(
+      compiler.options.entry,
+      getAdditionalEntries({ options: this.options, devServer: compiler.options.devServer })
+    );
 
     // Inject necessary modules to bundle's global scope
     /** @type {Record<string, string>} */
diff --git a/lib/utils/getAdditionalEntries.js b/lib/utils/getAdditionalEntries.js
new file mode 100644
index 0000000..1cc525b
--- /dev/null
+++ b/lib/utils/getAdditionalEntries.js
@@ -0,0 +1,62 @@
+const querystring = require('querystring');
+
+/** @typedef {{ prependEntries: import('./injectRefreshEntry').WebpackEntry, overlayEntries: import('./injectRefreshEntry').WebpackEntry }} AdditionalEntries */
+
+/**
+ * Creates an object that contains two entry arrays: the prependEntries and overlayEntries
+ * @param {Object} optionsContainer This is the container for the options to this function
+ * @param {import('../types').NormalizedPluginOptions} optionsContainer.options Configuration options for this plugin.
+ * @param {import('webpack').Compiler.options.devServer} [optionsContainer.devServer] The webpack devServer config
+ * @returns {AdditionalEntries} An object that contains the Webpack entries for prepending and the overlay feature
+ */
+function getAdditionalEntries({ options, devServer }) {
+  /** @type {Record<string, *>} */
+  let resourceQuery = {};
+
+  if (devServer) {
+    const { sockHost, sockPath, sockPort, port, host, https, http2 } = devServer;
+
+    (sockHost || host) && (resourceQuery.sockHost = sockHost ? sockHost : host);
+    sockPath && (resourceQuery.sockPath = sockPath);
+    (sockPort || port) && (resourceQuery.sockPort = sockPort ? sockPort : port);
+    resourceQuery.sockProtocol = https || http2 ? 'https' : 'http';
+  }
+
+  if (options.overlay) {
+    options.overlay.sockHost && (resourceQuery.sockHost = options.overlay.sockHost);
+    options.overlay.sockPath && (resourceQuery.sockPath = options.overlay.sockPath);
+    options.overlay.sockPort && (resourceQuery.sockPort = options.overlay.sockPort);
+    options.overlay.sockProtocol && (resourceQuery.sockProtocol = options.overlay.sockProtocol);
+  }
+
+  // We don't need to URI encode the resourceQuery as it will be parsed by Webpack
+  const queryString = querystring.stringify(resourceQuery, undefined, undefined, {
+    /**
+     * @param {string} string
+     * @returns {string}
+     */
+    encodeURIComponent(string) {
+      return string;
+    },
+  });
+
+  const prependEntries = [
+    // React-refresh runtime
+    require.resolve('../../client/ReactRefreshEntry'),
+  ];
+
+  const overlayEntries = [
+    // Legacy WDS SockJS integration
+    options.overlay &&
+      options.overlay.useLegacyWDSSockets &&
+      require.resolve('../../client/LegacyWDSSocketEntry'),
+    // Error overlay runtime
+    options.overlay &&
+      options.overlay.entry &&
+      `${options.overlay.entry}${queryString ? `?${queryString}` : ''}`,
+  ].filter(Boolean);
+
+  return { prependEntries, overlayEntries };
+}
+
+module.exports = getAdditionalEntries;
diff --git a/lib/utils/index.js b/lib/utils/index.js
index 6644bf5..f98b6ba 100644
--- a/lib/utils/index.js
+++ b/lib/utils/index.js
@@ -5,6 +5,7 @@ const getSocketIntegration = require('./getSocketIntegration');
 const injectRefreshEntry = require('./injectRefreshEntry');
 const injectRefreshLoader = require('./injectRefreshLoader');
 const normalizeOptions = require('./normalizeOptions');
+const getAdditionalEntries = require('./getAdditionalEntries');
 
 module.exports = {
   createError,
@@ -14,4 +15,5 @@ module.exports = {
   injectRefreshEntry,
   injectRefreshLoader,
   normalizeOptions,
+  getAdditionalEntries,
 };
diff --git a/lib/utils/injectRefreshEntry.js b/lib/utils/injectRefreshEntry.js
index 4808f43..638544b 100644
--- a/lib/utils/injectRefreshEntry.js
+++ b/lib/utils/injectRefreshEntry.js
@@ -1,4 +1,3 @@
-const querystring = require('querystring');
 const createError = require('./createError');
 
 /** @typedef {string | string[] | import('webpack').Entry} StaticEntry */
@@ -28,45 +27,11 @@ function isSocketEntry(entry) {
 /**
  * Injects an entry to the bundle for react-refresh.
  * @param {WebpackEntry} [originalEntry] A Webpack entry object.
- * @param {import('../types').NormalizedPluginOptions} options Configuration options for this plugin.
+ * @param {import('./getAdditionalEntries').AdditionalEntries} additionalEntries An object that contains the Webpack entries for prepending and the overlay feature
  * @returns {WebpackEntry} An injected entry object.
  */
-function injectRefreshEntry(originalEntry, options) {
-  /** @type {Record<string, *>} */
-  let resourceQuery = {};
-  if (options.overlay) {
-    options.overlay.sockHost && (resourceQuery.sockHost = options.overlay.sockHost);
-    options.overlay.sockPath && (resourceQuery.sockPath = options.overlay.sockPath);
-    options.overlay.sockPort && (resourceQuery.sockPort = options.overlay.sockPort);
-    options.overlay.sockProtocol && (resourceQuery.sockProtocol = options.overlay.sockProtocol);
-  }
-
-  // We don't need to URI encode the resourceQuery as it will be parsed by Webpack
-  const queryString = querystring.stringify(resourceQuery, undefined, undefined, {
-    /**
-     * @param {string} string
-     * @returns {string}
-     */
-    encodeURIComponent(string) {
-      return string;
-    },
-  });
-
-  const prependEntries = [
-    // React-refresh runtime
-    require.resolve('../../client/ReactRefreshEntry'),
-  ];
-
-  const overlayEntries = [
-    // Legacy WDS SockJS integration
-    options.overlay &&
-      options.overlay.useLegacyWDSSockets &&
-      require.resolve('../../client/LegacyWDSSocketEntry'),
-    // Error overlay runtime
-    options.overlay &&
-      options.overlay.entry &&
-      options.overlay.entry + (queryString && `?${queryString}`),
-  ].filter(Boolean);
+function injectRefreshEntry(originalEntry, additionalEntries) {
+  const { prependEntries = [], overlayEntries = [] } = additionalEntries;
 
   // Single string entry point
   if (typeof originalEntry === 'string') {
@@ -96,9 +61,9 @@ function injectRefreshEntry(originalEntry, options) {
           typeof curEntry === 'object' && curEntry.import
             ? {
                 ...curEntry,
-                import: injectRefreshEntry(curEntry.import, options),
+                import: injectRefreshEntry(curEntry.import, additionalEntries),
               }
-            : injectRefreshEntry(curEntry, options),
+            : injectRefreshEntry(curEntry, additionalEntries),
       }),
       {}
     );
@@ -107,7 +72,7 @@ function injectRefreshEntry(originalEntry, options) {
   if (typeof originalEntry === 'function') {
     return (...args) =>
       Promise.resolve(originalEntry(...args)).then((resolvedEntry) =>
-        injectRefreshEntry(resolvedEntry, options)
+        injectRefreshEntry(resolvedEntry, additionalEntries)
       );
   }
 
diff --git a/sockets/utils/getCurrentScriptSource.js b/sockets/utils/getCurrentScriptSource.js
index 9fb4f44..e282cd2 100644
--- a/sockets/utils/getCurrentScriptSource.js
+++ b/sockets/utils/getCurrentScriptSource.js
@@ -15,8 +15,6 @@ function getCurrentScriptSource() {
   if (currentScript) {
     return currentScript.getAttribute('src');
   }
-
-  throw new Error('[React Refresh] Failed to get current script source!');
 }
 
 module.exports = getCurrentScriptSource;
diff --git a/sockets/utils/getSocketUrlParts.js b/sockets/utils/getSocketUrlParts.js
index 3c909b2..f47098d 100644
--- a/sockets/utils/getSocketUrlParts.js
+++ b/sockets/utils/getSocketUrlParts.js
@@ -19,7 +19,7 @@ const parseQuery = require('./parseQuery');
  */
 function getSocketUrlParts(resourceQuery) {
   const scriptSource = getCurrentScriptSource();
-  const urlParts = url.parse(scriptSource);
+  const urlParts = url.parse(scriptSource || '');
 
   /** @type {string | undefined} */
   let auth;
@@ -87,6 +87,17 @@ function getSocketUrlParts(resourceQuery) {
     protocol = parsedQuery.sockProtocol + ':';
   }
 
+  if (!hostname || !pathname || !port) {
+    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,


Test patch:
diff --git a/test/unit/getAdditionalEntries.test.js b/test/unit/getAdditionalEntries.test.js
new file mode 100644
index 0000000..947c2cc
--- /dev/null
+++ b/test/unit/getAdditionalEntries.test.js
@@ -0,0 +1,160 @@
+const getAdditionalEntries = require('../../lib/utils/getAdditionalEntries');
+
+const ErrorOverlayEntry = require.resolve('../../client/ErrorOverlayEntry');
+const ReactRefreshEntry = require.resolve('../../client/ReactRefreshEntry');
+
+const DEFAULT_OPTIONS = {
+  overlay: {
+    entry: ErrorOverlayEntry,
+  },
+};
+
+describe('getAdditionalEntries', () => {
+  it('should work with default settings', () => {
+    expect(getAdditionalEntries({ options: DEFAULT_OPTIONS })).toStrictEqual({
+      overlayEntries: [ErrorOverlayEntry],
+      prependEntries: [ReactRefreshEntry],
+    });
+  });
+
+  it('should append legacy WDS entry when required', () => {
+    expect(
+      getAdditionalEntries({
+        options: {
+          overlay: {
+            entry: ErrorOverlayEntry,
+            useLegacyWDSSockets: true,
+          },
+        },
+      })
+    ).toStrictEqual({
+      overlayEntries: [require.resolve('../../client/LegacyWDSSocketEntry'), ErrorOverlayEntry],
+      prependEntries: [ReactRefreshEntry],
+    });
+  });
+
+  it('should append resource queries to the overlay entry when specified in overlay options', () => {
+    expect(
+      getAdditionalEntries({
+        options: {
+          overlay: {
+            entry: ErrorOverlayEntry,
+            sockHost: 'localhost',
+            sockPath: '/socket',
+            sockPort: '9000',
+            sockProtocol: 'https',
+          },
+        },
+      })
+    ).toStrictEqual({
+      prependEntries: [ReactRefreshEntry],
+      overlayEntries: [
+        `${ErrorOverlayEntry}?sockHost=localhost&sockPath=/socket&sockPort=9000&sockProtocol=https`,
+      ],
+    });
+  });
+
+  it('should append resource queries to the overlay entry when specified in devServer options', () => {
+    expect(
+      getAdditionalEntries({
+        options: DEFAULT_OPTIONS,
+        devServer: {
+          sockHost: 'localhost',
+          sockPath: '/socket',
+          sockPort: '9000',
+        },
+      })
+    ).toStrictEqual({
+      prependEntries: [ReactRefreshEntry],
+      overlayEntries: [
+        `${ErrorOverlayEntry}?sockHost=localhost&sockPath=/socket&sockPort=9000&sockProtocol=http`,
+      ],
+    });
+  });
+
+  it('should correctly set sock protocol when https is specified in devServer options', () => {
+    expect(
+      getAdditionalEntries({
+        options: DEFAULT_OPTIONS,
+        devServer: {
+          https: true,
+        },
+      })
+    ).toStrictEqual({
+      prependEntries: [ReactRefreshEntry],
+      overlayEntries: [`${ErrorOverlayEntry}?sockProtocol=https`],
+    });
+  });
+
+  it('should correctly set sock protocol when http2 is specified in devServer options', () => {
+    expect(
+      getAdditionalEntries({
+        options: DEFAULT_OPTIONS,
+        devServer: {
+          http2: true,
+        },
+      })
+    ).toStrictEqual({
+      prependEntries: [ReactRefreshEntry],
+      overlayEntries: [`${ErrorOverlayEntry}?sockProtocol=https`],
+    });
+  });
+
+  it('should use overlay options over devServer options with both are specified', () => {
+    expect(
+      getAdditionalEntries({
+        options: {
+          overlay: {
+            entry: ErrorOverlayEntry,
+            sockHost: 'localhost',
+            sockPath: '/socket',
+            sockPort: '9000',
+            sockProtocol: 'https',
+          },
+        },
+        devServer: {
+          sockHost: 'someotherhost',
+          sockPath: '/socketpath',
+          sockPort: '9001',
+        },
+      })
+    ).toStrictEqual({
+      prependEntries: [ReactRefreshEntry],
+      overlayEntries: [
+        `${ErrorOverlayEntry}?sockHost=localhost&sockPath=/socket&sockPort=9000&sockProtocol=https`,
+      ],
+    });
+  });
+
+  it('should use devServer port and host as fallbacks if sockPort and sockHost are not defined', () => {
+    expect(
+      getAdditionalEntries({
+        options: DEFAULT_OPTIONS,
+        devServer: {
+          port: 8888,
+          host: 'localhost',
+        },
+      })
+    ).toStrictEqual({
+      prependEntries: [ReactRefreshEntry],
+      overlayEntries: [`${ErrorOverlayEntry}?sockHost=localhost&sockPort=8888&sockProtocol=http`],
+    });
+  });
+
+  it('should use the devServer sockHost and sockPort options over port and host if they are available', () => {
+    expect(
+      getAdditionalEntries({
+        options: DEFAULT_OPTIONS,
+        devServer: {
+          port: 8888,
+          host: 'otherhost',
+          sockPort: 9999,
+          sockHost: 'localhost',
+        },
+      })
+    ).toStrictEqual({
+      prependEntries: [ReactRefreshEntry],
+      overlayEntries: [`${ErrorOverlayEntry}?sockHost=localhost&sockPort=9999&sockProtocol=http`],
+    });
+  });
+});
diff --git a/test/unit/getSocketUrlParts.test.js b/test/unit/getSocketUrlParts.test.js
index 600315e..1a73431 100644
--- a/test/unit/getSocketUrlParts.test.js
+++ b/test/unit/getSocketUrlParts.test.js
@@ -176,6 +176,18 @@ describe('getSocketUrlParts', () => {
     });
   });
 
+  it('should use info from resource query if script source is unavailable', () => {
+    getCurrentScriptSource.mockImplementationOnce(() => null);
+
+    expect(getSocketUrlParts('?sockHost=foo.com&sockPath=/socket&sockPort=9000')).toStrictEqual({
+      auth: undefined,
+      hostname: 'foo.com',
+      pathname: '/socket',
+      port: '9000',
+      protocol: 'http:',
+    });
+  });
+
   it('should use info from resource query when available', () => {
     getCurrentScriptSource.mockImplementationOnce(() => 'http://localhost:8080');
 
@@ -189,4 +201,12 @@ describe('getSocketUrlParts', () => {
       protocol: 'https:',
     });
   });
+
+  it('should throw if script source and resource query are not defined', () => {
+    getCurrentScriptSource.mockImplementationOnce(() => null);
+
+    expect(() => getSocketUrlParts(null)).toThrow(
+      '[React Refresh] Failed to get an URL for the socket connection.'
+    );
+  });
 });
diff --git a/test/unit/injectRefreshEntry.test.js b/test/unit/injectRefreshEntry.test.js
index c4ff236..3def983 100644
--- a/test/unit/injectRefreshEntry.test.js
+++ b/test/unit/injectRefreshEntry.test.js
@@ -3,15 +3,14 @@ const injectRefreshEntry = require('../../lib/utils/injectRefreshEntry');
 const ErrorOverlayEntry = require.resolve('../../client/ErrorOverlayEntry');
 const ReactRefreshEntry = require.resolve('../../client/ReactRefreshEntry');
 
-const DEFAULT_OPTIONS = {
-  overlay: {
-    entry: ErrorOverlayEntry,
-  },
+const DEFAULT_ENTRIES = {
+  overlayEntries: [ErrorOverlayEntry],
+  prependEntries: [ReactRefreshEntry],
 };
 
 describe('injectRefreshEntry', () => {
   it('should add entries to a string', () => {
-    expect(injectRefreshEntry('test.js', DEFAULT_OPTIONS)).toStrictEqual([
+    expect(injectRefreshEntry('test.js', DEFAULT_ENTRIES)).toStrictEqual([
       ReactRefreshEntry,
       ErrorOverlayEntry,
       'test.js',
@@ -19,7 +18,7 @@ describe('injectRefreshEntry', () => {
   });
 
   it('should add entries to an array', () => {
-    expect(injectRefreshEntry(['test.js'], DEFAULT_OPTIONS)).toStrictEqual([
+    expect(injectRefreshEntry(['test.js'], DEFAULT_ENTRIES)).toStrictEqual([
       ReactRefreshEntry,
       ErrorOverlayEntry,
       'test.js',
@@ -33,7 +32,7 @@ describe('injectRefreshEntry', () => {
           main: 'test.js',
           vendor: ['react', 'react-dom'],
         },
-        DEFAULT_OPTIONS
+        DEFAULT_ENTRIES
       )
     ).toStrictEqual({
       main: [ReactRefreshEntry, ErrorOverlayEntry, 'test.js'],
@@ -54,7 +53,7 @@ describe('injectRefreshEntry', () => {
             },
             vendor: ['react', 'react-dom'],
           },
-          DEFAULT_OPTIONS
+          DEFAULT_ENTRIES
         )
       ).toStrictEqual({
         main: {
@@ -67,7 +66,7 @@ describe('injectRefreshEntry', () => {
   );
 
   it('should add entries to a synchronous function', () => {
-    const returnedEntry = injectRefreshEntry(() => 'test.js', DEFAULT_OPTIONS);
+    const returnedEntry = injectRefreshEntry(() => 'test.js', DEFAULT_ENTRIES);
     expect(typeof returnedEntry).toBe('function');
     expect(returnedEntry()).resolves.toStrictEqual([
       ReactRefreshEntry,
@@ -77,7 +76,7 @@ describe('injectRefreshEntry', () => {
   });
 
   it('should add entries to an asynchronous function', () => {
-    const returnedEntry = injectRefreshEntry(() => Promise.resolve('test.js'), DEFAULT_OPTIONS);
+    const returnedEntry = injectRefreshEntry(() => Promise.resolve('test.js'), DEFAULT_ENTRIES);
     expect(typeof returnedEntry).toBe('function');
     expect(returnedEntry()).resolves.toStrictEqual([
       ReactRefreshEntry,
@@ -92,7 +91,7 @@ describe('injectRefreshEntry', () => {
         main: 'test.js',
         vendor: ['react', 'react-dom'],
       }),
-      DEFAULT_OPTIONS
+      DEFAULT_ENTRIES
     );
     expect(typeof returnedEntry).toBe('function');
     expect(returnedEntry()).resolves.toStrictEqual({
@@ -102,45 +101,11 @@ describe('injectRefreshEntry', () => {
   });
 
   it('should not append overlay entry when unused', () => {
-    expect(injectRefreshEntry('test.js', {})).toStrictEqual([ReactRefreshEntry, 'test.js']);
-  });
-
-  it('should append legacy WDS entry when required', () => {
-    expect(
-      injectRefreshEntry('test.js', {
-        overlay: {
-          entry: ErrorOverlayEntry,
-          useLegacyWDSSockets: true,
-        },
-      })
-    ).toStrictEqual([
-      ReactRefreshEntry,
-      require.resolve('../../client/LegacyWDSSocketEntry'),
-      ErrorOverlayEntry,
-      'test.js',
-    ]);
-  });
-
-  it('should append resource queries to the overlay entry when specified', () => {
-    expect(
-      injectRefreshEntry('test.js', {
-        overlay: {
-          entry: ErrorOverlayEntry,
-          sockHost: 'localhost',
-          sockPath: '/socket',
-          sockPort: '9000',
-          sockProtocol: 'https',
-        },
-      })
-    ).toStrictEqual([
-      ReactRefreshEntry,
-      `${ErrorOverlayEntry}?sockHost=localhost&sockPath=/socket&sockPort=9000&sockProtocol=https`,
-      'test.js',
-    ]);
+    expect(injectRefreshEntry('test.js', {})).toStrictEqual(['test.js']);
   });
 
   it('should append overlay entry for a string after socket-related entries', () => {
-    expect(injectRefreshEntry('webpack-dev-server/client', DEFAULT_OPTIONS)).toStrictEqual([
+    expect(injectRefreshEntry('webpack-dev-server/client', DEFAULT_ENTRIES)).toStrictEqual([
       ReactRefreshEntry,
       'webpack-dev-server/client',
       ErrorOverlayEntry,
@@ -149,7 +114,7 @@ describe('injectRefreshEntry', () => {
 
   it('should append overlay entry for an array after socket-related entries while keeping original relative order', () => {
     expect(
-      injectRefreshEntry(['setup-env.js', 'webpack-dev-server/client', 'test.js'], DEFAULT_OPTIONS)
+      injectRefreshEntry(['setup-env.js', 'webpack-dev-server/client', 'test.js'], DEFAULT_ENTRIES)
     ).toStrictEqual([
       ReactRefreshEntry,
       'setup-env.js',
@@ -160,7 +125,7 @@ describe('injectRefreshEntry', () => {
   });
 
   it('should throw when non-parsable entry is received', () => {
-    expect(() => injectRefreshEntry(1, DEFAULT_OPTIONS)).toThrowErrorMatchingInlineSnapshot(
+    expect(() => injectRefreshEntry(1, DEFAULT_ENTRIES)).toThrowErrorMatchingInlineSnapshot(
       `"[React Refresh] Failed to parse the Webpack \`entry\` object!"`
     );
   });