Left some code comments

Summary: Added some additional code comments as requested in earlier diffs

Reviewed By: passy

Differential Revision: D33361691

fbshipit-source-id: 2d1adc8830af58e6cb7bac4b283a5c0171cf749e

desktop/flipper-server/src/startBaseServer.tsx

@@ -68,8 +68,16 @@ function addWebsocket(server: http.Server, config: Config) {
     allowRequest(req, callback) {
       const noOriginHeader = req.headers.origin === undefined;
       if (noOriginHeader && req.headers.host === validHost) {
+        // no origin header? Either the request is not cross-origin,
+        // or the request is not originating from a browser, so should be OK to pass through
         callback(null, true);
       } else {
+        // for now we don't allow cross origin request, so that an arbitrary website cannot try to
+        // connect a socket to localhost:serverport, and try to use the all powerful Flipper APIs to read
+        // for example files.
+        // Potentially in the future we do want to allow this, e.g. if we want to connect to a local flipper-server
+        // directly from intern. But before that, we should either authenticate the request somehow,
+        // and discuss security impact and for example scope the files that can be read by Flipper.
         console.warn(
           `Refused sockect connection from cross domain request, origin: ${req.headers.origin}, host: ${req.headers.host}. Expected: ${validHost}`,
         );

desktop/flipper-server/src/startWebServerDev.tsx

@@ -108,6 +108,11 @@ export async function startWebServerDev(
         if (moduleName === 'flipper') {
           return MetroResolver.resolve(context, 'flipper-ui-core', ...rest);
         }
+        // stubbed modules are modules that don't make sense outside a Node / Electron context,
+        // like fs, child_process etc etc.
+        // UI / plugins using these features should use the corresponding RenderHost api's instead
+        // Ideally we'd fail hard on those, but not all plugins are properly converted yet, and some
+        // libraries try to require them for feature detection (e.g. jsbase64)
         if (stubModules.has(moduleName)) {
           console.warn(
             `Found a reference to built-in module '${moduleName}', which will be stubbed out. Referer: ${context.originModulePath}`,

desktop/scripts/build-utils.ts

@@ -477,6 +477,11 @@ export async function buildBrowserBundle(outDir: string, dev: boolean) {
         if (moduleName === 'flipper') {
           return MetroResolver.resolve(context, 'flipper-ui-core', ...rest);
         }
+        // stubbed modules are modules that don't make sense outside a Node / Electron context,
+        // like fs, child_process etc etc.
+        // UI / plugins using these features should use the corresponding RenderHost api's instead
+        // Ideally we'd fail hard on those, but not all plugins are properly converted yet, and some
+        // libraries try to require them for feature detection (e.g. jsbase64)
         if (stubModules.has(moduleName)) {
           console.warn(
             `Found a reference to built-in module '${moduleName}', which will be stubbed out. Referer: ${context.originModulePath}`,

desktop/scripts/start-flipper-server-dev.ts

@@ -145,6 +145,8 @@ async function startWatchChanges() {
   try {
     const watchman = new Watchman(path.resolve(__dirname, '..'));
     await watchman.initialize();
+    // We only watch for changes that might affect the server.
+    // For UI changes, Metro / hot module reloading / fast refresh take care of the changes
     await Promise.all(
       [
         'pkg',