Disable errors for duplicate emitted file names #3175
Conversation
marijnh
force-pushed
the
no-duplicate-filename-error
branch
2 times, most recently
from
3100b4e
to
3c9ee28
Compare
|
There were two tests checking for the presence of this error. I've updated the patch to skip them. |
Codecov Report
@@ Coverage Diff @@
## master #3175 +/- ##
==========================================
- Coverage 90.2% 90.19% -0.01%
==========================================
Files 165 165
Lines 5910 5907 -3
Branches 1800 1799 -1
==========================================
- Hits 5331 5328 -3
Misses 350 350
Partials 229 229
Continue to review full report at Codecov.
|
src/utils/FileEmitter.ts
Outdated
| @@ -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 | |||
There was a problem hiding this comment.
From my side, I would eventually go for a warning instead of an error.
| @@ -1,4 +1,5 @@ | |||
| module.exports = { | |||
| skip: true, | |||
There was a problem hiding this comment.
I would actually just delete the tests altogether. Otherwise there is a high danger that the tests rot, and we could restore them from git anyway. If we switch to a warning, the tests need to be rewritten anyway.
marijnh
force-pushed
the
no-duplicate-filename-error
branch
from
69043e4
to
c8d4dbd
Compare
|
Updated the patch to remove those tests entirely (I set them to skip since it is my experience that it's easy to forget restoring removed tests). I'm not sure what do do about your comment about the comment content. |
And add a test to make sure such emits no longer error spuriously. Issue rollup#3174
* Add test description * Describe better in the TODO comment what the ultimate intention is
lukastaegert
force-pushed
the
no-duplicate-filename-error
branch
from
c8d4dbd
to
d9421ef
Compare
|
Thanks!
Personally I would just write new tests, especially since the combinatorics are not too complicated, the new tests should test for a warning instead of an error and tests that do not run as part of the full pipeline quickly become forgotten and meaningless. |
No problem, I adjusted the comment slightly. |
|
Thank you! |
And add a test to make sure such emits no longer error spuriously.
Issue #3174
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
#3174
Description
This works around an issue produced by a race condition in the plugin driver when multiple outputs are generated concurrently. Different outputs emitting the same file name via
emitFilewould fail. @lukastaegert indicated he's planning a proper fix for this, but in the meantime, this unblocks ezolenko/rollup-plugin-typescript2#180 (comment) by just turning off the error that was being reported incorrectly.