[Flight] Prevent non-Server imports of aliased Server entrypoints (#2…

…0422) * [Flight] Prevent non-Server imports of aliased Server entrypoints * Fix Flow + await * Tighten the types
facebook · Dec 10, 2020 · 40ff239 · 40ff239
1 parent 94aa365
commit 40ff239
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 16 deletions.
diff --git a/fixtures/flight-browser/index.html b/fixtures/flight-browser/index.html
@@ -19,7 +19,7 @@ <h1>Flight Example</h1>
     <script src="../../build/node_modules/react/umd/react.development.js"></script>
     <script src="../../build/node_modules/react-dom/umd/react-dom.development.js"></script>
     <script src="../../build/node_modules/react-dom/umd/react-dom-server.browser.development.js"></script>
-    <script src="../../build/node_modules/react-server-dom-webpack/umd/react-server-dom-webpack-writer.browser.server.development.js"></script>
+    <script src="../../build/node_modules/react-server-dom-webpack/umd/react-server-dom-webpack-writer.browser.development.server.js"></script>
     <script src="../../build/node_modules/react-server-dom-webpack/umd/react-server-dom-webpack.development.js"></script>
     <script src="https://unpkg.com/babel-standalone@6/babel.js"></script>
     <script type="text/babel">

diff --git a/packages/react-server-dom-webpack/src/ReactFlightWebpackNodeLoader.js b/packages/react-server-dom-webpack/src/ReactFlightWebpackNodeLoader.js
@@ -18,7 +18,7 @@ type ResolveFunction = (
   string,
   ResolveContext,
   ResolveFunction,
-) => Promise<{url: string}>;
+) => {url: string} | Promise<{url: string}>;
 
 type GetSourceContext = {
   format: string,
@@ -70,19 +70,24 @@ export async function resolve(
       );
     }
   }
-  // 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')) {
+  const resolved = await defaultResolve(specifier, context, defaultResolve);
+  if (resolved.url.endsWith('.server.js')) {
+    const parentURL = context.parentURL;
+    if (parentURL && !parentURL.endsWith('.server.js')) {
+      let reason;
+      if (specifier.endsWith('.server.js')) {
+        reason = `"${specifier}"`;
+      } else {
+        reason = `"${specifier}" (which expands to "${resolved.url}")`;
+      }
       throw new Error(
-        `Cannot import "${specifier}" from "${context.parentURL}". ` +
+        `Cannot import ${reason} from "${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);
+  return resolved;
 }
 
 export async function getSource(
@@ -128,7 +133,7 @@ function addExportNames(names, node) {
 function resolveClientImport(
   specifier: string,
   parentURL: string,
-): Promise<{url: string}> {
+): {url: string} | Promise<{url: string}> {
   // Resolve an import specifier as if it was loaded by the client. This doesn't use
   // the overrides that this loader does but instead reverts to the default.
   // This resolution algorithm will not necessarily have the same configuration

diff --git a/packages/react-server-dom-webpack/src/ReactFlightWebpackNodeRegister.js b/packages/react-server-dom-webpack/src/ReactFlightWebpackNodeRegister.js
@@ -70,22 +70,26 @@ module.exports = function register() {
   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')) {
+    const resolved = originalResolveFilename.apply(this, arguments);
+    if (resolved.endsWith('.server.js')) {
       if (
         parent &&
         parent.filename &&
         !parent.filename.endsWith('.server.js')
       ) {
+        let reason;
+        if (request.endsWith('.server.js')) {
+          reason = `"${request}"`;
+        } else {
+          reason = `"${request}" (which expands to "${resolved}")`;
+        }
         throw new Error(
-          `Cannot import "${request}" from "${parent.filename}". ` +
+          `Cannot import ${reason} 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);
+    return resolved;
   };
 };