Disallow *.server.js imports from any other files (#20309)

sebmarkbage · web-flow · commit 565148d7516f · 2020-11-30T14:25:56.000-08:00
This convention ensures that you can declare that you intend for a file
to only be used on the server (even if it technically might resolve
on the client).
diff --git a/fixtures/flight/server/cli.server.js b/fixtures/flight/server/cli.server.js
diff --git a/fixtures/flight/server/package.json b/fixtures/flight/server/package.json
@@ -1,3 +1,4 @@
 {
-  "type": "commonjs"
+  "type": "commonjs",
+  "main": "./cli.server.js"
 }
diff --git a/packages/react-transport-dom-webpack/src/ReactFlightWebpackNodeLoader.js b/packages/react-transport-dom-webpack/src/ReactFlightWebpackNodeLoader.js
@@ -31,12 +31,39 @@ type GetSourceFunction = (
 
 type Source = string | ArrayBuffer | Uint8Array;
 
+let warnedAboutConditionsFlag = false;
+
 export async function resolve(
   specifier: string,
   context: ResolveContext,
   defaultResolve: ResolveFunction,
 ): Promise<string> {
-  // TODO: Resolve server-only files.
+  if (!context.conditions.includes('react-server')) {
+    context = {
+      ...context,
+      conditions: [...context.conditions, 'react-server'],
+    };
+    if (!warnedAboutConditionsFlag) {
+      warnedAboutConditionsFlag = true;
+      // eslint-disable-next-line react-internal/no-production-logging
+      console.warn(
+        'You did not run Node.js with the `--conditions react-server` flag. ' +
+          'Any "react-server" override will only work with ESM imports.',
+      );
+    }
+  }
+  // We intentionally check the specifier here instead of the resolved file.
+  // This allows package exports to configure non-server aliases that resolve to server files
+  // depending on environment. It's probably a bad idea to export a server file as "main" though.
+  if (specifier.endsWith('.server.js')) {
+    if (context.parentURL && !context.parentURL.endsWith('.server.js')) {
+      throw new Error(
+        `Cannot import "${specifier}" from "${context.parentURL}". ` +
+          'By react-server convention, .server.js files can only be imported from other .server.js files. ' +
+          'That way nobody accidentally sends these to the client by indirectly importing it.',
+      );
+    }
+  }
   return defaultResolve(specifier, context, defaultResolve);
 }
 
diff --git a/packages/react-transport-dom-webpack/src/ReactFlightWebpackNodeRegister.js b/packages/react-transport-dom-webpack/src/ReactFlightWebpackNodeRegister.js
@@ -9,11 +9,36 @@
 
 const url = require('url');
 
+// $FlowFixMe
+const Module = require('module');
+
 module.exports = function register() {
   (require: any).extensions['.client.js'] = function(module, path) {
     module.exports = {
       $$typeof: Symbol.for('react.module.reference'),
       name: url.pathToFileURL(path).href,
     };
   };
+
+  const originalResolveFilename = Module._resolveFilename;
+
+  Module._resolveFilename = function(request, parent, isMain, options) {
+    // We intentionally check the request here instead of the resolved file.
+    // This allows package exports to configure non-server aliases that resolve to server files
+    // depending on environment. It's probably a bad idea to export a server file as "main" though.
+    if (request.endsWith('.server.js')) {
+      if (
+        parent &&
+        parent.filename &&
+        !parent.filename.endsWith('.server.js')
+      ) {
+        throw new Error(
+          `Cannot import "${request}" from "${parent.filename}". ` +
+            'By react-server convention, .server.js files can only be imported from other .server.js files. ' +
+            'That way nobody accidentally sends these to the client by indirectly importing it.',
+        );
+      }
+    }
+    return originalResolveFilename.apply(this, arguments);
+  };
 };
diff --git a/scripts/rollup/bundles.js b/scripts/rollup/bundles.js
@@ -310,7 +310,7 @@ const bundles = [
     moduleType: RENDERER_UTILS,
     entry: 'react-transport-dom-webpack/node-register',
     global: 'ReactFlightWebpackNodeRegister',
-    externals: ['url'],
+    externals: ['url', 'module'],
   },
 
   /******* React Transport DOM Server Relay *******/
