Skip to content

Commit

Permalink
Disallow *.server.js imports from any other files (#20309)
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
sebmarkbage committed Nov 30, 2020
1 parent e6a0f27 commit 565148d
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 3 deletions.
File renamed without changes.
3 changes: 2 additions & 1 deletion fixtures/flight/server/package.json
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
{
"type": "commonjs"
"type": "commonjs",
"main": "./cli.server.js"
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
};
2 changes: 1 addition & 1 deletion scripts/rollup/bundles.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 *******/
Expand Down

0 comments on commit 565148d

Please sign in to comment.