Fixed error in Profiling Plugin, if given folder path doesn't exist. #8565
Conversation
|
For maintainers only:
|
|
Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon. |
lib/debug/ProfilingPlugin.js
Outdated
| const trace = new Tracer({ | ||
| noStream: true | ||
| }); | ||
| const profiler = new Profiler(inspector); | ||
| if (outputPath.match(/\/|\\/)) { |
There was a problem hiding this comment.
| if (outputPath.match(/\/|\\/)) { | |
| if (/\/|\\/.test(outputPath)) { |
But I think this isn't needed as it's always true.
There was a problem hiding this comment.
This results to false, when they don't send any outputPath. It defaults to "events.json" as the outputpath.
lib/debug/ProfilingPlugin.js
Outdated
| const trace = new Tracer({ | ||
| noStream: true | ||
| }); | ||
| const profiler = new Profiler(inspector); | ||
| if (outputPath.match(/\/|\\/)) { | ||
| const dir = path.dirname(outputPath); | ||
| compiler.outputFileSystem.mkdirp(dir); |
There was a problem hiding this comment.
Here it would becomes a bit tricky to get the async correctly.
I think it's fine to use the sync version here. You should not use the outputFileSystem here, but the real fs. Best import mkdirp and call mkdirp.sync.
There was a problem hiding this comment.
Made changes as requested. Thanks @sokra for your suggestions.
Would like to learn even more during this process and contribute to Webpack.
|
@NaviMarella Thanks for your update. I labeled the Pull Request so reviewers will review it again. @sokra Please review the new changes. |
|
Thanks |
Fixed error in Profiling Plugin, if given folder path doesn't exist.
This is a fix for the below issue. Profiling plugin will create a folder based on the output path, if it doesn't exists.
#8503
What kind of change does this PR introduce?
bugfix
Did you add tests for your changes?
Yes, i have added a test for checking if folder is getting created in the provided path.
Does this PR introduce a breaking change?
No
What needs to be documented once your changes are merged?
No documentation needed.