-
-
Notifications
You must be signed in to change notification settings - Fork 622
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: only set output path on passing flag #1855
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/cc @webpack/cli-team
Good work!
@anshumanv Thanks for your update. I labeled the Pull Request so reviewers will review it again. @snitin315 Please review the new changes. |
Currently the build/test fails with the following output: ``` Test webextension-polyfill bundled with webpack =============================================== [webpack-cli] Unknown argument: --output [webpack-cli] Unknown argument: /tmp/webpack-bundle.js ? Which flags do you want to use? … ✔ --entry: The entry point(s) of your application e.g. ./src/main.js ✔ --config: Provide path to a webpack configuration file e.g. ./webpack.config.js ✔ --color: Enable/Disable colors on console ✔ --merge: Merge two or more configurations using webpack-merge e.g. -c ./webpack.config.js -c ./webpack.test.config.js --merge ✔ --progress: Print compilation progress during build ✔ --help: Outputs list of supported flags ✔ --output-path: Output location of the file generated by webpack e.g. ./dist/ ``` Regressed by: webpack/webpack-cli#1855 webpack/webpack-cli@2f36b9d The `-o` flag is compatible with both the old and new flag name.
This should have been called It is a breaking change, and quite a late one to make after beta, before RC... |
Actually, it seems that |
@Rob--W you have configurations so you can setup this, it is breaking change and described |
Currently the build/test fails with the following output: ``` Test webextension-polyfill bundled with webpack =============================================== [webpack-cli] Unknown argument: --output [webpack-cli] Unknown argument: /tmp/webpack-bundle.js ? Which flags do you want to use? … ✔ --entry: The entry point(s) of your application e.g. ./src/main.js ✔ --config: Provide path to a webpack configuration file e.g. ./webpack.config.js ✔ --color: Enable/Disable colors on console ✔ --merge: Merge two or more configurations using webpack-merge e.g. -c ./webpack.config.js -c ./webpack.test.config.js --merge ✔ --progress: Print compilation progress during build ✔ --help: Outputs list of supported flags ✔ --output-path: Output location of the file generated by webpack e.g. ./dist/ ``` Regressed by: webpack/webpack-cli#1855 webpack/webpack-cli@2f36b9d `--output` has been renamed to `--output-path` (`-o` in both cases), but stopped supporting file names. To output to a specific absolute path, `--output-path` and `--output-filename` must both be specified, with the directory in the former and the filename in the latter.
It seems that there is no way to pass an absolute file path via the command line any more, is there? This limitation is strange though. Most tools that I can recall supporting a parameter to an output file accept an absolute path, whereas this tool now requires to separate flags. Is there any reason for this change? |
@Rob--W because path and filename is different things, you can use placeholders in filename, so we don't know in |
The meaning of the If you consciously accepted these breaking, backwards-incompatible changes, then feel free to ignore my comment. |
Currently the build/test fails with the following output: ``` Test webextension-polyfill bundled with webpack =============================================== [webpack-cli] Unknown argument: --output [webpack-cli] Unknown argument: /tmp/webpack-bundle.js ? Which flags do you want to use? … ✔ --entry: The entry point(s) of your application e.g. ./src/main.js ✔ --config: Provide path to a webpack configuration file e.g. ./webpack.config.js ✔ --color: Enable/Disable colors on console ✔ --merge: Merge two or more configurations using webpack-merge e.g. -c ./webpack.config.js -c ./webpack.test.config.js --merge ✔ --progress: Print compilation progress during build ✔ --help: Outputs list of supported flags ✔ --output-path: Output location of the file generated by webpack e.g. ./dist/ ``` Regressed by: webpack/webpack-cli#1855 webpack/webpack-cli@2f36b9d `--output` has been renamed to `--output-path` (`-o` in both cases), but stopped supporting file names. To output to a specific absolute path, `--output-path` and `--output-filename` must both be specified, with the directory in the former and the filename in the latter.
Currently the build/test fails with the following output: ``` Test webextension-polyfill bundled with webpack =============================================== [webpack-cli] Unknown argument: --output [webpack-cli] Unknown argument: /tmp/webpack-bundle.js ? Which flags do you want to use? … ✔ --entry: The entry point(s) of your application e.g. ./src/main.js ✔ --config: Provide path to a webpack configuration file e.g. ./webpack.config.js ✔ --color: Enable/Disable colors on console ✔ --merge: Merge two or more configurations using webpack-merge e.g. -c ./webpack.config.js -c ./webpack.test.config.js --merge ✔ --progress: Print compilation progress during build ✔ --help: Outputs list of supported flags ✔ --output-path: Output location of the file generated by webpack e.g. ./dist/ ``` Regressed by: webpack/webpack-cli#1855 webpack/webpack-cli@2f36b9d `--output` has been renamed to `--output-path` (`-o` in both cases), but stopped supporting file names. To output to a specific absolute path, `--output-path` and `--output-filename` must both be specified, with the directory in the former and the filename in the latter.
What kind of change does this PR introduce?
output
tooutput-path
Did you add tests for your changes?
Yes
If relevant, did you update the documentation?
Yes
Summary
Previously we were assigning output file and path which was not correct so fixed that and converted output flag to output-path
Does this PR introduce a breaking change?
Yes
Other information
Fixes #1844