fix(webpack-plugin): Ensure asset relocator injected code works with …

…nodeIntegration disabled (#2396) * Adjust injected code when nodeIntegration = false Co-authored-by: Mark Lee <electronjs@lazymalevolence.com>
electron · Jul 23, 2021 · 146fc31 · 146fc31
1 parent 760382c
commit 146fc31
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 11 deletions.
diff --git a/packages/plugin/webpack/src/WebpackConfig.ts b/packages/plugin/webpack/src/WebpackConfig.ts
@@ -201,7 +201,10 @@ export default class WebpackConfigGenerator {
         filename: `${entryPoint.name}/index.html`,
         chunks: [entryPoint.name].concat(entryPoint.additionalChunks || []),
       }) as WebpackPluginInstance).concat(
-        [new webpack.DefinePlugin(defines), new AssetRelocatorPatch(this.isProd)],
+        [
+          new webpack.DefinePlugin(defines),
+          new AssetRelocatorPatch(this.isProd, !!this.pluginConfig.renderer.nodeIntegration),
+        ],
       );
     return webpackMerge({
       entry,

diff --git a/packages/plugin/webpack/src/util/AssetRelocatorPatch.ts b/packages/plugin/webpack/src/util/AssetRelocatorPatch.ts
@@ -3,8 +3,26 @@ import { Chunk, Compiler } from 'webpack';
 export default class AssetRelocatorPatch {
   private readonly isProd: boolean;
 
-  constructor(isProd: boolean) {
+  private readonly nodeIntegration: boolean;
+
+  constructor(isProd: boolean, nodeIntegration: boolean) {
     this.isProd = isProd;
+    this.nodeIntegration = nodeIntegration;
+  }
+
+  private injectedProductionDirnameCode(): string {
+    if (this.nodeIntegration) {
+      // In production the assets are found one directory up from
+      // __dirname
+      //
+      // __dirname cannot be used directly until this PR lands
+      // https://github.com/jantimon/html-webpack-plugin/pull/1650
+      return 'require("path").resolve(require("path").dirname(__filename), "..")';
+    }
+
+    // If nodeIntegration is disabled, we replace __dirname
+    // with an empty string so no error is thrown at runtime
+    return '""';
   }
 
   public apply(compiler: Compiler) {
@@ -32,20 +50,17 @@ export default class AssetRelocatorPatch {
                   throw new Error('The installed version of @vercel/webpack-asset-relocator-loader does not appear to be compatible with Forge');
                 }
 
+                if (this.isProd) {
+                  return originalInjectCode.replace('__dirname', this.injectedProductionDirnameCode());
+                }
+
                 return originalInjectCode.replace(
                   '__dirname',
-                  this.isProd
-                  // In production the assets are found one directory up from
-                  // __dirname
-                  //
-                  // __dirname cannot be used directly until this PR lands
-                  // https://github.com/jantimon/html-webpack-plugin/pull/1650
-                    ? 'require("path").resolve(require("path").dirname(__filename), "..")'
                   // In development, the app is loaded via webpack-dev-server
                   // so __dirname is useless because it points to Electron
                   // internal code. Instead we hard-code the absolute path to
                   // the webpack output.
-                    : JSON.stringify(compiler.options.output.path),
+                  JSON.stringify(compiler.options.output.path),
                 );
               };
             }

diff --git a/packages/plugin/webpack/test/AssetRelocatorPatch_spec.ts b/packages/plugin/webpack/test/AssetRelocatorPatch_spec.ts
@@ -109,6 +109,7 @@ describe('AssetRelocatorPatch', () => {
   const config = {
     mainConfig: './webpack.main.config.js',
     renderer: {
+      nodeIntegration: true,
       config: './webpack.renderer.config.js',
       entryPoints: [
         {
@@ -176,7 +177,7 @@ describe('AssetRelocatorPatch', () => {
   });
 
   describe('Production', () => {
-    const generator = new WebpackConfigGenerator(config, appPath, true, 3000);
+    let generator = new WebpackConfigGenerator(config, appPath, true, 3000);
 
     it('builds main', async () => {
       const mainConfig = generator.getMainConfig();
@@ -224,5 +225,20 @@ describe('AssetRelocatorPatch', () => {
       expect(output).to.contain('Hello, world! from the preload');
       expect(output).to.contain('Hello, world! from the renderer');
     });
+
+    it('builds renderer with nodeIntegration = false', async () => {
+      config.renderer.nodeIntegration = false;
+      generator = new WebpackConfigGenerator(config, appPath, true, 3000);
+
+      const rendererConfig = await generator.getRendererConfig(config.renderer.entryPoints);
+      await asyncWebpack(rendererConfig);
+
+      await expectOutputFileToHaveTheCorrectNativeModulePath({
+        outDir: rendererOut,
+        jsPath: path.join(rendererOut, 'main_window/index.js'),
+        nativeModulesString: '.ab="/native_modules/"',
+        nativePathString: `.ab+"${nativePathSuffix}"`,
+      });
+    });
   });
 });