Update the documentation of wp scripts #14499
Conversation
Jackie6
requested review from
aduth,
ajitbohra,
gziolo,
mkaz,
nerrad,
oandregal,
ntwb and
youknowriad
as code owners
| @@ -54,6 +54,12 @@ This is how you execute the script with presented setup: | |||
|
|
|||
| * `npm run build` - builds the code for production. | |||
|
|
|||
| _Options_: | |||
|
|
|||
| - `--WP_BUNDLE_ANALYZER` : This flag is used to enables utility that represents bundle content as convenient interactive zoomable treemap. | |||
There was a problem hiding this comment.
As I understand it, these are interpreted by the wp scripts script as environment variables, not as options flags.
In other words, instead of:
wp scripts build --WP_BUNDLE_ANALYZER
I think the correct usage is...
WP_BUNDLE_ANALYZER=true wp scripts
Please correct me if I'm mistaken.
There was a problem hiding this comment.
Hi! Thanks for comments!
I am not sure which way is correct to use --WP_BUNDLE_ANALYZER. But for the WP_LIVE_RELOAD_PORT, I have tried npm run build --WP_LIVE_RELOAD_PORT=12345 and npm run build WP_LIVE_RELOAD_PORT=12345. The first one works but the second one fails and the error message is
Insufficient number of arguments or no entry found. Alternatively, run 'webpack(-cli) --help' for usage info.
According to the documentation of Environment Variables and Environment Options in web pack, I think we can use the double hyphen to specify the variables including WP_BUNDLE_ANALYZER, WP_BUNDLE_ANALYZER and WP_DEVTOOL.
There was a problem hiding this comment.
@aduth is correct, they should be rather provided before the command. Unless we replicate what @draganescu did for e2e tests and introduce new CLI args, convert them to env variables in node and filter them out before passing to webpack.
gziolo
added
[Type] Developer Documentation
| @@ -96,11 +96,7 @@ const config = { | |||
| ], | |||
| }, | |||
| plugins: [ | |||
| // WP_BUNDLE_ANALYZER global variable enables utility that represents bundle content | |||
There was a problem hiding this comment.
These inline comments should stay here.
|
|
||
| - `--WP_BUNDLE_ANALYZER` : This flag is used to enables utility that represents bundle content as convenient interactive zoomable treemap. | ||
| - `--WP_LIVE_RELOAD_PORT` : When present, the port on which live reload works when running watch mode will be changed. An example usage would be `npx wp-scripts build --WP_LIVE_RELOAD_PORT=1234`. | ||
| - `--WP_DEVTOOL` : A development tool is designated to control how source maps are generated. More information can be found [here](https://webpack.js.org/configuration/devtool/#devtool). |
There was a problem hiding this comment.
Both WP_DEVTOOL and WP_LIVE_RELOAD_PORT get only applied in development mode - this means it works only with start script.
| @@ -54,6 +54,12 @@ This is how you execute the script with presented setup: | |||
|
|
|||
| * `npm run build` - builds the code for production. | |||
|
|
|||
| _Options_: | |||
|
|
|||
| - `--WP_BUNDLE_ANALYZER` : This flag is used to enables utility that represents bundle content as convenient interactive zoomable treemap. | |||
There was a problem hiding this comment.
@aduth is correct, they should be rather provided before the command. Unless we replicate what @draganescu did for e2e tests and introduce new CLI args, convert them to env variables in node and filter them out before passing to webpack.
There was a problem hiding this comment.
Thanks for the time invested in this proposal.
As per conversation in the core-js meetings, the consensus is to explore how plugins/themes can extend the default webpack config. That's something we haven't considered yet AFAIK, and this method may not be the one we end up implementing (or perhaps it is! I don't know). I worry that if we make it public now, we'll be tied to this, so until we have time to revisit how to better extend the webpack defaults, I'd rather not document these internal things.
This is something that is already supported by |
|
@Jackie6 do you plan to address feedback shared? |
|
There is a similar logic implemented for |
|
@gziolo Hey Greg. Honestly speaking, I am not very familiar with how these environment variables work. This pr could be closed so that others may fix this issue. I am sorry about that. |
Thanks for your response and starting this PR. Let's close it at suggested so someone else could give it a try :) |
Description
Fix #14492
How has this been tested?
Types of changes
Update the documentation of wp scripts. Specifically, add the following options to
wp-scripts buildand remove some redundant comments inwebpack.config.js.WP_BUNDLE_ANALYZER
WP_DEVTOOL
WP_LIVE_RELOAD_PORT
Checklist: