From 65a01fe1b93f22419a888e6122405b1738ec588f Mon Sep 17 00:00:00 2001 From: Marijn Haverbeke Date: Fri, 18 Oct 2019 17:54:17 +0200 Subject: [PATCH 1/2] Disable errors for duplicate emitted file names And add a test to make sure such emits no longer error spuriously. Issue #3174 --- src/utils/FileEmitter.ts | 4 +-- .../samples/emit-same-file/_config.js | 12 +++++++ .../emit-same-file/_expected/amd/main.js | 9 ++++++ .../emit-same-file/_expected/amd/myfile | 1 + .../emit-same-file/_expected/cjs/main.js | 7 +++++ .../emit-same-file/_expected/cjs/myfile | 1 + .../emit-same-file/_expected/es/main.js | 3 ++ .../emit-same-file/_expected/es/myfile | 1 + .../emit-same-file/_expected/system/main.js | 12 +++++++ .../emit-same-file/_expected/system/myfile | 1 + .../samples/emit-same-file/main.js | 1 + .../_config.js | 31 ------------------- .../main.js | 1 - .../emit-named-chunk-conflict/_config.js | 25 --------------- .../emit-named-chunk-conflict/buildStart1.js | 1 - .../emit-named-chunk-conflict/buildStart2.js | 1 - .../emit-named-chunk-conflict/main.js | 1 - 17 files changed, 50 insertions(+), 62 deletions(-) create mode 100644 test/chunking-form/samples/emit-same-file/_config.js create mode 100644 test/chunking-form/samples/emit-same-file/_expected/amd/main.js create mode 100644 test/chunking-form/samples/emit-same-file/_expected/amd/myfile create mode 100644 test/chunking-form/samples/emit-same-file/_expected/cjs/main.js create mode 100644 test/chunking-form/samples/emit-same-file/_expected/cjs/myfile create mode 100644 test/chunking-form/samples/emit-same-file/_expected/es/main.js create mode 100644 test/chunking-form/samples/emit-same-file/_expected/es/myfile create mode 100644 test/chunking-form/samples/emit-same-file/_expected/system/main.js create mode 100644 test/chunking-form/samples/emit-same-file/_expected/system/myfile create mode 100644 test/chunking-form/samples/emit-same-file/main.js delete mode 100644 test/function/samples/emit-file/emit-named-asset-conflict-build-generate/_config.js delete mode 100644 test/function/samples/emit-file/emit-named-asset-conflict-build-generate/main.js delete mode 100644 test/function/samples/emit-file/emit-named-chunk-conflict/_config.js delete mode 100644 test/function/samples/emit-file/emit-named-chunk-conflict/buildStart1.js delete mode 100644 test/function/samples/emit-file/emit-named-chunk-conflict/buildStart2.js delete mode 100644 test/function/samples/emit-file/emit-named-chunk-conflict/main.js diff --git a/src/utils/FileEmitter.ts b/src/utils/FileEmitter.ts index 0b29ae20748..8bab92e8ff4 100644 --- a/src/utils/FileEmitter.ts +++ b/src/utils/FileEmitter.ts @@ -10,7 +10,6 @@ import { errAssetSourceAlreadySet, errChunkNotGeneratedForFileName, errFailedValidation, - errFileNameConflict, errFileReferenceIdNotFoundForFilename, errInvalidRollupPhaseForChunkEmission, errNoAssetSourceSet, @@ -50,7 +49,8 @@ function generateAssetFileName( function reserveFileNameInBundle(fileName: string, bundle: OutputBundleWithPlaceholders) { if (fileName in bundle) { - return error(errFileNameConflict(fileName)); + // FIXME this should return error(errFileNameConflict(fileName)); + // but until #3174 is fixed, this raises spurious errors and is disabled } bundle[fileName] = FILE_PLACEHOLDER; } diff --git a/test/chunking-form/samples/emit-same-file/_config.js b/test/chunking-form/samples/emit-same-file/_config.js new file mode 100644 index 00000000000..af34504e833 --- /dev/null +++ b/test/chunking-form/samples/emit-same-file/_config.js @@ -0,0 +1,12 @@ +module.exports = { + options: { + input: 'main.js', + plugins: [ + { + generateBundle() { + this.emitFile({ type: 'asset', fileName: 'myfile', source: 'abc' }); + } + } + ] + } +}; diff --git a/test/chunking-form/samples/emit-same-file/_expected/amd/main.js b/test/chunking-form/samples/emit-same-file/_expected/amd/main.js new file mode 100644 index 00000000000..f99c5d38fdf --- /dev/null +++ b/test/chunking-form/samples/emit-same-file/_expected/amd/main.js @@ -0,0 +1,9 @@ +define(['exports'], function (exports) { 'use strict'; + + function hi() { return 2 } + + exports.hi = hi; + + Object.defineProperty(exports, '__esModule', { value: true }); + +}); diff --git a/test/chunking-form/samples/emit-same-file/_expected/amd/myfile b/test/chunking-form/samples/emit-same-file/_expected/amd/myfile new file mode 100644 index 00000000000..f2ba8f84ab5 --- /dev/null +++ b/test/chunking-form/samples/emit-same-file/_expected/amd/myfile @@ -0,0 +1 @@ +abc \ No newline at end of file diff --git a/test/chunking-form/samples/emit-same-file/_expected/cjs/main.js b/test/chunking-form/samples/emit-same-file/_expected/cjs/main.js new file mode 100644 index 00000000000..ce637f0dcc8 --- /dev/null +++ b/test/chunking-form/samples/emit-same-file/_expected/cjs/main.js @@ -0,0 +1,7 @@ +'use strict'; + +Object.defineProperty(exports, '__esModule', { value: true }); + +function hi() { return 2 } + +exports.hi = hi; diff --git a/test/chunking-form/samples/emit-same-file/_expected/cjs/myfile b/test/chunking-form/samples/emit-same-file/_expected/cjs/myfile new file mode 100644 index 00000000000..f2ba8f84ab5 --- /dev/null +++ b/test/chunking-form/samples/emit-same-file/_expected/cjs/myfile @@ -0,0 +1 @@ +abc \ No newline at end of file diff --git a/test/chunking-form/samples/emit-same-file/_expected/es/main.js b/test/chunking-form/samples/emit-same-file/_expected/es/main.js new file mode 100644 index 00000000000..47c2f790880 --- /dev/null +++ b/test/chunking-form/samples/emit-same-file/_expected/es/main.js @@ -0,0 +1,3 @@ +function hi() { return 2 } + +export { hi }; diff --git a/test/chunking-form/samples/emit-same-file/_expected/es/myfile b/test/chunking-form/samples/emit-same-file/_expected/es/myfile new file mode 100644 index 00000000000..f2ba8f84ab5 --- /dev/null +++ b/test/chunking-form/samples/emit-same-file/_expected/es/myfile @@ -0,0 +1 @@ +abc \ No newline at end of file diff --git a/test/chunking-form/samples/emit-same-file/_expected/system/main.js b/test/chunking-form/samples/emit-same-file/_expected/system/main.js new file mode 100644 index 00000000000..0581de90888 --- /dev/null +++ b/test/chunking-form/samples/emit-same-file/_expected/system/main.js @@ -0,0 +1,12 @@ +System.register([], function (exports) { + 'use strict'; + return { + execute: function () { + + exports('hi', hi); + + function hi() { return 2 } + + } + }; +}); diff --git a/test/chunking-form/samples/emit-same-file/_expected/system/myfile b/test/chunking-form/samples/emit-same-file/_expected/system/myfile new file mode 100644 index 00000000000..f2ba8f84ab5 --- /dev/null +++ b/test/chunking-form/samples/emit-same-file/_expected/system/myfile @@ -0,0 +1 @@ +abc \ No newline at end of file diff --git a/test/chunking-form/samples/emit-same-file/main.js b/test/chunking-form/samples/emit-same-file/main.js new file mode 100644 index 00000000000..be03a2f1d9f --- /dev/null +++ b/test/chunking-form/samples/emit-same-file/main.js @@ -0,0 +1 @@ +export function hi() { return 2 } diff --git a/test/function/samples/emit-file/emit-named-asset-conflict-build-generate/_config.js b/test/function/samples/emit-file/emit-named-asset-conflict-build-generate/_config.js deleted file mode 100644 index c148dcc7912..00000000000 --- a/test/function/samples/emit-file/emit-named-asset-conflict-build-generate/_config.js +++ /dev/null @@ -1,31 +0,0 @@ -module.exports = { - description: - 'throws when there is a conflict between two named assets emitted during different phases', - options: { - input: 'main', - plugins: { - buildStart() { - this.emitFile({ - type: 'asset', - fileName: 'custom/emitted.txt', - source: 'text' - }); - }, - renderStart() { - this.emitFile({ - type: 'asset', - fileName: 'custom/emitted.txt', - source: 'text' - }); - } - } - }, - generateError: { - code: 'PLUGIN_ERROR', - hook: 'renderStart', - message: - 'Could not emit file "custom/emitted.txt" as it conflicts with an already emitted file.', - plugin: 'at position 1', - pluginCode: 'FILE_NAME_CONFLICT' - } -}; diff --git a/test/function/samples/emit-file/emit-named-asset-conflict-build-generate/main.js b/test/function/samples/emit-file/emit-named-asset-conflict-build-generate/main.js deleted file mode 100644 index c0b933d7b56..00000000000 --- a/test/function/samples/emit-file/emit-named-asset-conflict-build-generate/main.js +++ /dev/null @@ -1 +0,0 @@ -console.log('main'); diff --git a/test/function/samples/emit-file/emit-named-chunk-conflict/_config.js b/test/function/samples/emit-file/emit-named-chunk-conflict/_config.js deleted file mode 100644 index dfc45b3f266..00000000000 --- a/test/function/samples/emit-file/emit-named-chunk-conflict/_config.js +++ /dev/null @@ -1,25 +0,0 @@ -module.exports = { - description: 'throws when there is a conflict between two emitted named chunks', - options: { - input: 'main', - plugins: { - buildStart() { - this.emitFile({ - type: 'chunk', - id: 'buildStart1', - fileName: 'custom/build-start-chunk.js' - }); - this.emitFile({ - type: 'chunk', - id: 'buildStart2', - fileName: 'custom/build-start-chunk.js' - }); - } - } - }, - generateError: { - code: 'FILE_NAME_CONFLICT', - message: - 'Could not emit file "custom/build-start-chunk.js" as it conflicts with an already emitted file.' - } -}; diff --git a/test/function/samples/emit-file/emit-named-chunk-conflict/buildStart1.js b/test/function/samples/emit-file/emit-named-chunk-conflict/buildStart1.js deleted file mode 100644 index 4c326cae1bc..00000000000 --- a/test/function/samples/emit-file/emit-named-chunk-conflict/buildStart1.js +++ /dev/null @@ -1 +0,0 @@ -console.log('buildStart1'); diff --git a/test/function/samples/emit-file/emit-named-chunk-conflict/buildStart2.js b/test/function/samples/emit-file/emit-named-chunk-conflict/buildStart2.js deleted file mode 100644 index a74fcff5302..00000000000 --- a/test/function/samples/emit-file/emit-named-chunk-conflict/buildStart2.js +++ /dev/null @@ -1 +0,0 @@ -console.log('buildStart2'); diff --git a/test/function/samples/emit-file/emit-named-chunk-conflict/main.js b/test/function/samples/emit-file/emit-named-chunk-conflict/main.js deleted file mode 100644 index c0b933d7b56..00000000000 --- a/test/function/samples/emit-file/emit-named-chunk-conflict/main.js +++ /dev/null @@ -1 +0,0 @@ -console.log('main'); From d9421ef0134a1506aa36900d79c6c21fe6461b82 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Sun, 20 Oct 2019 19:39:47 +0200 Subject: [PATCH 2/2] * Make sure test is red with original code * Add test description * Describe better in the TODO comment what the ultimate intention is --- src/utils/FileEmitter.ts | 6 ++---- test/chunking-form/samples/emit-same-file/_config.js | 3 +++ 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/utils/FileEmitter.ts b/src/utils/FileEmitter.ts index 8bab92e8ff4..c6196285fc0 100644 --- a/src/utils/FileEmitter.ts +++ b/src/utils/FileEmitter.ts @@ -48,10 +48,8 @@ function generateAssetFileName( } function reserveFileNameInBundle(fileName: string, bundle: OutputBundleWithPlaceholders) { - if (fileName in bundle) { - // FIXME this should return error(errFileNameConflict(fileName)); - // but until #3174 is fixed, this raises spurious errors and is disabled - } + // TODO this should warn if the fileName is already in the bundle, + // but until #3174 is fixed, this raises spurious warnings and is disabled bundle[fileName] = FILE_PLACEHOLDER; } diff --git a/test/chunking-form/samples/emit-same-file/_config.js b/test/chunking-form/samples/emit-same-file/_config.js index af34504e833..bad65a0f0d7 100644 --- a/test/chunking-form/samples/emit-same-file/_config.js +++ b/test/chunking-form/samples/emit-same-file/_config.js @@ -1,10 +1,13 @@ module.exports = { + description: + 'does not throw an error if multiple files with the same name are emitted (until #3174 is fixed)', options: { input: 'main.js', plugins: [ { generateBundle() { this.emitFile({ type: 'asset', fileName: 'myfile', source: 'abc' }); + this.emitFile({ type: 'asset', fileName: 'myfile', source: 'abc' }); } } ]