-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
WP Scripts: Automatically detect entry points for the default JS and CSS files #66466
base: trunk
Are you sure you want to change the base?
Conversation
Size Change: 0 B Total Size: 1.82 MB ℹ️ View Unchanged
|
30223c6
to
068e305
Compare
2c1d41c
to
8c25bad
Compare
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @xsonic, @james0r, @wasek23, @ghostintranslation, @daotw. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
I included the changelog entry with 7ce5696. I marked changes as breaking as it's very hard to predict whether all existing projects will work precisely the same. However, I put as much care as I could into understanding the implications to minimize the risk. |
Sorry if my question is out of line, I kind of lost track about how all these tools work over time. But why does So here's a proposal that may or may not be valid depending on the answer to the question above but what if we change that and require an entry point to the build command:
What use-cases we would be missing in this case? |
@youknowriad, great questions. I must admit that given how long the WP Scripts tool is around, I also don’t fully remember which conventions we established in which order and when we over optimized for plugins with blocks 😅 What’s interesting webpack 4 supported the syntax you mentioned: wp-scripts build index.js That’s why we added a compatibility layer on top of webpack 5 to retain the functionality. Unfortunately, it doesn’t work with CSS or block metadata. What’s interesting, when you pass as many paths to files as you want they are used as entry points and we don’t use any automatic discovery for anything else. What would be compatible with your perspective is extending this format to support more asset types. Example: wp-scripts build index.ts style.scss **/block.json This is more or less the equivalent of the default behavior that I was trying to cover with the changes proposed. However, I’m happy to explore the alternative where you can pass paths to JS, CSS and block metadata explicitly of with globs that are further processed by the bundler and converted into entry points based on existing rules. I omitted The most complex setup I have seen we would need to potentially cover is multiple plugins with multiple blocks, and multiple themes in one project. That’s rather something that requires a custom configuration, but the approach you shared would allow a lot of flexibility in collecting all necessary entry points by listing files in the command. |
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.
This looks good, I left a couple notes, specifically I'd avoid supporting extensions lik .mjs
right now as that would be a nice quick way to support modules.
Yes, I think having a compatibility layer is fine as for me webpack is just an implementation details. I've never liked the fact that you can pass your webpack config and would prefer if we remove it entirely at some point but I guess it's ok to keep for now as it doesn't prevent us from exploring the possibility to pass any random entry point. I'd love to avoid any |
@youknowriad, I'd like to ensure we have some shared understanding of the general direction. We should aim towards making the A more comprehensive example where we could go: wp-scripts build index.ts editor.js style.scss **/block.json --source-path assets --output-path public How do you feel about the changes included in this PR? Should we leave it as and land it despite the minimal risk that some existing projects extending the default webpack config could be impacted when using the |
In this example, I don't know why we need "source-path" at all? But yes, your summary makes a lot of sense to me.
For me, if we strive to solve the former (the example you shared above), the current PR might be largely unnecessary. That said, if you feel like it's a good temporary solution, I don't mind it knowing that people will have to change later. For the breaking changes, I think it's ok as long as the upgrade path is clear. |
Excellent feedback, @youknowriad. Much appreciated. In the next few days, I will try to collect more feedback from folks actually using WP Scripts before deciding whether land these changes as the intermediate step or whether to go straight into the direction discussed above which will require more work.
We made some assumptions on how the project is structured in the past when using
These CLI args offer a way to change these defaults. However, after checking the variant when you explicitly pass the paths to JS files it indeed isn't necessary to provide the source folder. The only downside is wp-scripts build src/assets/js/index.js In that case, the build file would be put inside |
I agree with the direction that's described in the last few comments here. I find abstraction over webpack is awkward in this tool, the implementation details related to webpack leak through in many places, yet webpack isn't fully exposed either. This makes it difficult to iterate and experiment, for example with script modules where non-webpack-based tooling may provide better alternatives. One question I do have is regarding script modules. Right now, entrypoints can be passed as positional arguments and the tool knows how to do the right thing. This works with classic scripts, but there's no way to pass a JavaScript entrypoint that should be compiled as a script module. I don't have a great solution, I think we'll need to support an additional named argument that adds script module entrypoints. I hinted at using |
I was looking at usage examples of {
"scripts": {
"build": "wp-scripts build",
"build:custom": "wp-scripts build entry-one.js entry-two.js --output-path=custom",
"build:copy-php": "wp-scripts build --webpack-copy-php",
"build:custom-directory": "wp-scripts build --webpack-src-dir=custom-directory"
}
} What I found interesting is that the direction discussed is fully backward compatible, and it offers a unified interface that could also work with PHP files. In effect, {
"scripts": {
"build": "wp-scripts build index.js **/*.php --source-path=src --output-path=custom",
}
} @sirreal, I don't have a formed vision for improved script module support. It's definitely something that we have to explore as part of these efforts. The only way we can precisely assume script modules today is through
Yes, we should operate with the assumption that webpack is going to be replaced with Rspack, Vite, or something else in the near future. |
For scripts modules, we should also check how the bundlers do it. Most of them offer a |
I talked to @ndiego and @justintadlock about the idea proposed by @youknowriad in #66466 (comment). They shared examples of custom webpack configs they used in projects. I validated how much the new CLI syntax could remove the need to deal with webpack internals, and I got very positively surprised how quickly I could map it to the idea discussed. Here is a summary. Blog post exampleThis is how the revised command could look for the config overrides included in the blog post Getting and setting Block Binding values in the Editor. npx wp-scripts build js/editor=./resources/js/editor.js --output-path=public I was also surprised to learn that I could run the command successfully with the current version of WP Scripts. Note: I tried also the following Social Sharing block exampleFor the Social Sharing block, the command in the future would look as follows: npx wp-scripts build ./src/*/block.json ./src/*/utils.php Note: All paths by default would get resolved from the current working directory, so As of today, the following should work, too: npx wp-scripts build --webpack-copy-php Note: It would ensure all PHP files get copied from Block Visibility pluginAnother config is for Block Visibility. In the future, it could be handled with the following: npx wp-scripts build block-visibility-editor=./src/editor.js block-visibility-setting=./src/settings.js block-visibility-editor-styles=./src/styles/editor.scss block-visibility-contextual-indicator-styles=./src/styles/contextual-indicators.scss block-visibility-setting-styles=./src/styles/settings.scss The custom config is required today, because there is no support for CSS as entry points. This PR adds this concept. I'm not entirely sure how the file path syntax fits here. It might even work out of the box for the syntax provided above. It won't work without small tweaks for a simplified version like X3P0: Ideas themeA more real-world example of what is necessary to do with themes. Here’s the webpack.config.js. This one is very complex as it also handles fonts and media. Something like that might work one day: npm run build css/blocks/*/*=./resources/scss/blocks/*/*.scss js/editor=./resources/js/editor.js css/*=./resources/scss/*.scss ./resources/{media,fonts}/*.* Important note: I don’t know how far we can go with these globs. The challenge is mostly for the part that maps the paths to entry points. |
I decided to switch this PR to the draft state for now. Based on my most recent comment, I will work on smaller tasks to improve the existing CLI command instead of trying to improve the automatic detection for entry points. |
I would love to understand more why you had to use |
Yes, it's optional. I used it to make the paths shorter 😅 I updated my comment to address your concern. We shouldn't need to promote it so heavily as it might be confusing. Still, it might be a handful option in some cases. |
What?
Fixes #55936.
Related to #65602.
Improves the default webpack configuration to become more flexible in how entry points get detected. Primarily, make it possible for the main CSS files get detected in the source directory and set as an entry point. Secondly, make it more likely that the JavaScript main file is set as an entry point if it isn't directly tied to any block.
Why?
It's been reported multiple times that WP Scripts is too much tied to block development. The expectation it works out of the box when writing JavaScript for plugins or styles for themes. The good example is the latest blog post from @justintadlock on WordPress Developer Blog: Getting and setting Block Binding values in the Editor. In particular, this PR tries to mitigate the need for a custom webpack config in the project like:
Taking into account the project structure explained in the article:
With the changes proposed, the entry point could still be located in the
resources/js/
folder, but the file would need to get renamed toindex.js
. Then the build command would become:This would altogether remove the need for the custom config file.
An open question is whether we should also handle different names as entry points in addition to
index.js
. In this case,editor.js
could be the alternative worth considering.How?
It is based on the convention where JS file is
index.js
(following Node.js) and the CSS file isstyle.css
(main stylesheet in themes).Detection logic for the list of entry points with the updated webpack config. There are several steps:
wp-scripts build index.js
. Using this option will set entry points based on the list of the paths passed.block.json
files for scripts listed insideeditorScript
,script
andviewScript
fields. Ifblock.json
is located in the root of the source directory, the scanning ends for backward compatibility.src/index.*
JavaScript file when exists.src/style.*
CSS file when exists.Testing Instructions
npx wp-create-block example-block --no-wp-scripts cd example-block ../node_modules/.bin/wp-scripts build
This covers the single block example in which case, steps (3) and (4) outlined in the "How?" section should be skipped because
block.json
is located in the root of the source directory.The
build
folder looks as follows:Documentation changes
The above-mentioned and other possible configurations are explained in the updated README file, too.