fix: conflict error (#619)

WIStudent · web-flow · commit c08c2c8cf91d · 2021-10-04T11:53:46.000+02:00
* fix: conflict error
* Use Set to avoid unnecessary duplicated filenames
* Raise error instead of silently ignoring licenses
diff --git a/src/WebpackLicensePlugin.ts b/src/WebpackLicensePlugin.ts
@@ -13,6 +13,11 @@ import WebpackFileSystem from './WebpackFileSystem'
 
 const pluginName = 'WebpackLicensePlugin'
 
+interface ObservedCompiler {
+  name: string;
+  isChild: boolean;
+}
+
 /**
  * @todo "emit" vs "compilation" & "optimizeChunkAssets" hooks
  * @todo add banner to chunks? boolean option + banner formatter?
@@ -23,6 +28,10 @@ const pluginName = 'WebpackLicensePlugin'
  * @todo preferred license types on ambiguity (licenses array or spdx expression)
  */
 export default class WebpackLicensePlugin implements IWebpackPlugin {
+  private readonly filenames = new Set<string>()
+  private createdFiles = false
+  private observedCompilers: ObservedCompiler[] = []
+
   constructor(private pluginOptions: Partial<IPluginOptions> = {}) {}
 
   public apply(compiler: webpack.Compiler) {
@@ -31,14 +40,28 @@ export default class WebpackLicensePlugin implements IWebpackPlugin {
         'webpack-license-plugin',
         this.handleCompilation.bind(this, compiler)
       )
+      compiler.hooks.watchRun.tapAsync(
+        'webpack-license-plugin',
+        this.handleWatchRun.bind(this)
+      )
     } else if (typeof compiler.plugin !== 'undefined') {
       compiler.plugin(
         'compilation',
         this.handleCompilation.bind(this, compiler)
       )
+      compiler.plugin(
+        'watchRun',
+        this.handleWatchRun.bind(this)
+      )
     }
   }
 
+  public async handleWatchRun(_: unknown, callback: () => void) {
+    this.createdFiles = false
+    this.observedCompilers = []
+    callback()
+  }
+
   public handleCompilation(
     compiler: webpack.Compiler,
     compilation: webpack.compilation.Compilation
@@ -62,14 +85,38 @@ export default class WebpackLicensePlugin implements IWebpackPlugin {
     chunks: webpack.compilation.Chunk[],
     callback: () => void
   ) {
+    this.observedCompilers.push({
+      name: compilation.compiler.name,
+      isChild: compilation.compiler.isChild()
+    })
+
+    if (this.createdFiles) {
+      const observedCompilersMessage = this.observedCompilers.map(({ name, isChild }) => `compiler: ${name}, isChild: ${isChild}`).join('\n');
+      const errorMessage = `${pluginName}: Found licenses after license files were already created.\nIf you see this message, you ran into an edge case we thought would not happen. Please open an isssue at https://github.com/codepunkt/webpack-license-plugin/issues with details of your webpack configuration so we can invastigate it further.\n${observedCompilersMessage}`
+      compilation.errors.push(errorMessage)
+      callback()
+      return
+    }
+
+    if (!compilation.compiler.isChild()) {
+      this.createdFiles = true
+    }
+
     const alertAggregator = new WebpackAlertAggregator(compilation)
     const optionsProvider = new OptionsProvider(alertAggregator)
 
     const options = optionsProvider.getOptions(this.pluginOptions)
     alertAggregator.flushAlerts(pluginName)
 
     const chunkIterator = new WebpackChunkIterator()
-    const filenames = chunkIterator.iterateChunks(compilation, chunks)
+    for (const filename of chunkIterator.iterateChunks(compilation, chunks)) {
+      this.filenames.add(filename)
+    }
+
+    if (compilation.compiler.isChild()) {
+      callback()
+      return
+    }
 
     const fileSystem = new WebpackFileSystem(compiler.inputFileSystem)
     const packageJsonReader = new PackageJsonReader(fileSystem)
@@ -88,11 +135,9 @@ export default class WebpackLicensePlugin implements IWebpackPlugin {
       )
     )
 
-    await licenseFileWriter.writeLicenseFiles(filenames, options)
+    await licenseFileWriter.writeLicenseFiles([...this.filenames], options)
     alertAggregator.flushAlerts(pluginName)
 
-    if (callback) {
-      callback()
-    }
+    callback()
   }
 }
diff --git a/test/unit/WebpackLicensePlugin.test.ts b/test/unit/WebpackLicensePlugin.test.ts
@@ -1,15 +1,35 @@
+import type * as webpack from 'webpack'
+import LicenseFileWriter from '../../src/LicenseFileWriter'
+import WebpackChunkIterator from '../../src/WebpackChunkIterator'
 import WebpackLicensePlugin from '../../src/WebpackLicensePlugin'
-import webpack = require('webpack')
+
+jest.mock('../../src/LicenseFileWriter')
+jest.mock('../../src/WebpackChunkIterator')
 
 const MockCompiler = jest.fn<webpack.Compiler, any[]>((i) => i)
 const MockCompilation = jest.fn<webpack.compilation.Compilation, any[]>((i) => i)
 const MockChunk = jest.fn<webpack.compilation.Chunk, any[]>((i) => i)
 
 describe('WebpackLicensePlugin', () => {
+  beforeEach(() => {
+    (LicenseFileWriter as jest.Mock).mockReset();
+    (LicenseFileWriter as jest.Mock).mockImplementation(() => ({
+      writeLicenseFiles: jest.fn()
+    }));
+    (WebpackChunkIterator as jest.Mock).mockReset();
+    (WebpackChunkIterator as jest.Mock).mockImplementation(() => ({
+      iterateChunks: () => []
+    }))
+  })
+
   describe('apply', () => {
-    test('taps into compilation hook if hooks are defined', () => {
+    test('taps into compilation and watchRun hooks if hooks are defined', () => {
       const compiler = new MockCompiler({
-        hooks: { compilation: { tap: jest.fn() } },
+        hooks: { 
+          compilation: { tap: jest.fn() },
+          watchRun: { tapAsync: jest.fn() }
+         },
+          
       })
       const instance = new WebpackLicensePlugin({})
       instance.apply(compiler)
@@ -19,15 +39,21 @@ describe('WebpackLicensePlugin', () => {
         'webpack-license-plugin',
         expect.any(Function)
       )
+      expect(compiler.hooks.watchRun.tapAsync).toHaveBeenCalledTimes(1)
+      expect(compiler.hooks.watchRun.tapAsync).toHaveBeenCalledWith(
+        'webpack-license-plugin',
+        expect.any(Function)
+      )
     })
 
     test('plugs into compilation otherwise', () => {
       const compiler = new MockCompiler({ plugin: jest.fn() })
       const instance = new WebpackLicensePlugin()
       instance.apply(compiler)
 
-      expect(compiler.plugin).toHaveBeenCalledTimes(1)
+      expect(compiler.plugin).toHaveBeenCalledTimes(2)
       expect(compiler.plugin).toHaveBeenCalledWith('compilation', expect.any(Function))
+      expect(compiler.plugin).toHaveBeenCalledWith('watchRun', expect.any(Function))
     })
   })
 
@@ -57,18 +83,143 @@ describe('WebpackLicensePlugin', () => {
   })
 
   describe('handleChunkAssetOptimization', () => {
+    const createMockCompilation = (name: string, isChild:  boolean) => new MockCompilation({
+      assets: [],
+      errors: [],
+      warnings: [],
+      compiler: {
+        name,
+        isChild: () => isChild
+      }
+    })
+
     test('calls plugin mechanism callback when done', async () => {
       const instance = new WebpackLicensePlugin()
       const callback = jest.fn()
 
       await instance.handleChunkAssetOptimization(
         new MockCompiler({ inputFileSystem: 'a', options: { context: 'b' } }),
-        new MockCompilation({ assets: [], errors: [], warnings: [] }),
+        createMockCompilation('mockCompiler', false),
         [new MockChunk()],
         callback
       )
 
       expect(callback).toHaveBeenCalledTimes(1)
     })
+
+    test('calls writeLicenseFiles with all filenames', async () => {
+      (WebpackChunkIterator as jest.Mock).mockReset();
+      (WebpackChunkIterator as jest.Mock)
+        .mockImplementationOnce(() => ({
+          iterateChunks: () => ['filename1', 'filename2']
+        }))
+        .mockImplementationOnce(() => ({
+          iterateChunks: () => ['filename1', 'filename3', 'filename4']
+        }))
+
+      const writeLicenseFiles = jest.fn();
+      (LicenseFileWriter as jest.Mock).mockReset();
+      (LicenseFileWriter as jest.Mock).mockImplementationOnce(() => ({ writeLicenseFiles }))
+
+      const instance = new WebpackLicensePlugin()
+
+      const callback1 = jest.fn()
+      const mockCompilation1 = createMockCompilation('mockCompiler1', true)
+      await instance.handleChunkAssetOptimization(
+        new MockCompiler({ inputFileSystem: 'a', options: { context: 'b' } }),
+        mockCompilation1,
+        [new MockChunk()],
+        callback1
+      )
+      
+      const callback2 = jest.fn()
+      const mockCompilation2 = createMockCompilation('mockCompiler2', false)
+      await instance.handleChunkAssetOptimization(
+        new MockCompiler({ inputFileSystem: 'a', options: { context: 'b' } }),
+        mockCompilation2,
+        [new MockChunk()],
+        callback2
+      )
+
+      expect(callback1).toHaveBeenCalledTimes(1)
+      expect(callback2).toHaveBeenCalledTimes(1)
+
+      expect(mockCompilation1.errors).toEqual([])
+      expect(mockCompilation2.errors).toEqual([])
+
+      expect(writeLicenseFiles).toHaveBeenCalledWith(['filename1', 'filename2', 'filename3', 'filename4'], expect.anything())
+      expect(writeLicenseFiles).toHaveBeenCalledTimes(1)
+    })
+
+    test('pushes error if handleChunkAssetOptimization is called again after files were written', async () => {
+      const instance = new WebpackLicensePlugin()
+
+      const callback1 = jest.fn()
+      const mockCompilation1 = createMockCompilation('mockCompiler1', false)
+      await instance.handleChunkAssetOptimization(
+        new MockCompiler({ inputFileSystem: 'a', options: { context: 'b' } }),
+        mockCompilation1,
+        [new MockChunk()],
+        callback1
+      )
+
+      const callback2 = jest.fn()
+      const mockCompilation2 = createMockCompilation('mockCompiler2', true)
+      await instance.handleChunkAssetOptimization(
+        new MockCompiler({ inputFileSystem: 'a', options: { context: 'b' } }),
+        mockCompilation2,
+        [new MockChunk()],
+        callback2
+      )
+
+      expect(callback1).toHaveBeenCalledTimes(1)
+      expect(callback2).toHaveBeenCalledTimes(1)
+
+      expect(mockCompilation1.errors).toEqual([])
+      expect(mockCompilation2.errors).toEqual(["WebpackLicensePlugin: Found licenses after license files were already created.\nIf you see this message, you ran into an edge case we thought would not happen. Please open an isssue at https://github.com/codepunkt/webpack-license-plugin/issues with details of your webpack configuration so we can invastigate it further.\ncompiler: mockCompiler1, isChild: false\ncompiler: mockCompiler2, isChild: true"])
+    })
+
+    test('reset when handleWatchRun is called', async () => {
+      const instance = new WebpackLicensePlugin()
+
+      const callback1 = jest.fn()
+      const mockCompilation1 = createMockCompilation('mockCompiler1', false)
+      await instance.handleChunkAssetOptimization(
+        new MockCompiler({ inputFileSystem: 'a', options: { context: 'b' } }),
+        mockCompilation1,
+        [new MockChunk()],
+        callback1
+      )
+
+      const callbackWatchRun = jest.fn()
+      await instance.handleWatchRun(undefined, callbackWatchRun)
+
+      const callback2 = jest.fn()
+      const mockCompilation2 = createMockCompilation('mockCompiler2', false)
+      await instance.handleChunkAssetOptimization(
+        new MockCompiler({ inputFileSystem: 'a', options: { context: 'b' } }),
+        mockCompilation2,
+        [new MockChunk()],
+        callback2
+      )
+
+      const callback3 = jest.fn()
+      const mockCompilation3 = createMockCompilation('mockCompiler3', true)
+      await instance.handleChunkAssetOptimization(
+        new MockCompiler({ inputFileSystem: 'a', options: { context: 'b' } }),
+        mockCompilation3,
+        [new MockChunk()],
+        callback3
+      )
+
+      expect(callback1).toHaveBeenCalledTimes(1)
+      expect(callbackWatchRun).toHaveBeenCalledTimes(1)
+      expect(callback2).toHaveBeenCalledTimes(1)
+      expect(callback3).toHaveBeenCalledTimes(1)
+
+      expect(mockCompilation1.errors).toEqual([])
+      expect(mockCompilation2.errors).toEqual([])
+      expect(mockCompilation3.errors).toEqual(["WebpackLicensePlugin: Found licenses after license files were already created.\nIf you see this message, you ran into an edge case we thought would not happen. Please open an isssue at https://github.com/codepunkt/webpack-license-plugin/issues with details of your webpack configuration so we can invastigate it further.\ncompiler: mockCompiler2, isChild: false\ncompiler: mockCompiler3, isChild: true"])
+    })
   })
 })
