-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Build: Update Gutenberg => Core integration #10638
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
base: trunk
Are you sure you want to change the base?
Build: Update Gutenberg => Core integration #10638
Conversation
|
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: @1178653+wordpress-develop-pr-bot[bot]@users.noreply.github.com. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Looks like there's an infinite loop because wp-build use time based hashes which triggers the "commit unsaved changes" on each commit. I'm going to disable that workflow temporarily. |
a05abba to
13832c0
Compare
c64cf1b to
090e18e
Compare
090e18e to
4fd202c
Compare
0685789 to
66ad4cd
Compare
d03d49e to
cd57438
Compare
|
This is exciting! Also paves the way to run Gutenberg e2e tests :) |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
cd9bf2c to
eff7b55
Compare
|
Ok I think this is ready to land personally. I'd love reviews though :) |
| /src/wp-includes/class-wp-block-parser-block.php | ||
| /src/wp-includes/class-wp-block-parser-frame.php | ||
| /src/wp-includes/theme.json | ||
| /src/wp-includes/theme-i18n.json |
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.
Not sure if we need to manually add this to svnignore too
|
|
||
| env: | ||
| PUPPETEER_SKIP_DOWNLOAD: ${{ true }} | ||
| NODE_OPTIONS: --max-old-space-size=4096 |
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.
What's this for?
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.
Typescript building on MacOS consumes more memory than the default allocated memory to node processes. I recall we had to do something similar elsewhere. (Without it the build fails on MacOS on CI)
| 'wp-includes/blocks/post-template/editor-rtl.css', | ||
| 'wp-includes/blocks/post-template/editor-rtl.min.css', | ||
| 'wp-includes/js/dist/undo-manager.js', | ||
| 'wp-includes/js/dist/undo-manager.min.js', |
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 is no longer an old file?
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.
We do have an undo-manager script, so no, it's no longer an old file. That said, I didn't understand why it was here before, I think that script existed in Gutenberg for some time.
ellatrix
left a comment
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.
Works great and makes a lot of sense. Let's try it. 🙂
80ab406 to
e885192
Compare
|
I've started to take a look at this, but it's a pretty big PR and I need a bit more time. Is this a blocker for anything specific? Or is there room for a few more days to review? I'd love to do finish a proper review and testing. |
|
Can wait for a few more days. Maybe we should aim to land it early next week. |
e885192 to
91e322b
Compare
91e322b to
fe07bf3
Compare
|
I gave this a test run on my local env and it's working really well so far! I did have to point the env to the Without having looked extensively at the code yet, I love the direction! It will save us so much manual work in porting code from Gutenberg to Core ❤️ |
|
I thought |
Yes that's correct! The WORKING_DIR is set to src whenever the
With this PR, I wasn't able to get my env running or build it from the |
|
I just tried and confirmed that both |
|
@desrosj Hi Jonathan, did you have time to look at the PR. I'd love to avoid letting it sit for too long. Regardless, I'll be available for any follow-ups... |
|
Unfortunately, I did not get to look much further before heading out on my holiday break. One suggestion I was going to make was around the new files that are replacing old ones that did the same thing. For example, managing packages. I was going to suggest that those are added to version control using an While this is a new approach, this maintains the history of those files so that a visual comparison is available instead of just a sea of red and green in separate files. |
Thanks for quickly chiming in, I guess what you're saying is that we shouldn't git ignore the files that come from Gutenberg. I found that the previous approach of gitignoring some files (JS, CSS) and no others (PHP, JSON) was not very consistent and also created a lot of confusion of where the source of truth of these files is. I think with the changes in this PR, it's a lot more clear. These are external dependencies maintained in Gutenberg. |
|
Sorry, I'm not at my computer currently, so that was poorly worded without an example. That comment was specifically targeted at any files within the |
|
Ah I see :) Thanks for clarifying. It makes more sense now. I don't really think there's a 1 to 1 relationship with what was being done before though, so it's a bit hard to think of this as "file renames", it's a completely different pipeline. |
Trac ticket: https://core.trac.wordpress.org/ticket/64393
Summary
This PR changes WordPress Core's Gutenberg integration from npm packages to a checkout-and-build approach. Instead of syncing individual npm packages, Core now checks out the Gutenberg repository, builds it, and copies the build artifacts. This enables Core to use Gutenberg's advanced features like route-based navigation, full-page rendering, and the new Font Library. But also offers us the possibility to streamline more backports later (an example in this PR is the automatic copy of theme.json file). Check the issue for more details on the problem at hand.
New Build Pipeline
npm run gutenberg:checkoutclones Gutenberg at a specified refnpm run gutenberg:buildruns Gutenberg's build processnpm run gutenberg:copycopies and transforms build output to Corenpm run gutenberg:integrateruns all three stepsWhat Gets Copied
From Gutenberg build to Core:
/build/routes/→/src/wp-includes/build/routes//build/pages/→/src/wp-includes/build/pages//build/modules/→/src/wp-includes/js/dist/script-modules//build/scripts/→/src/wp-includes/js/dist//build/styles/→/src/wp-includes/css/dist//build/blocks/→/src/wp-includes/blocks/Path Transformations
The copy script transforms Gutenberg plugin paths to Core paths:
plugins_url()→includes_url()plugin_dir_path()→ABSPATH . WPINC . '/build/'Ideally these transformations shouldn't be needed, maybe we can make our "build" tool more generic to streamline the integration better and allow "building for core" directly. I'll look into that separately.
Webpack Changes
Removed webpack configs that are now replaced by Gutenberg's build:
tools/webpack/blocks.js- Using Gutenberg's block buildstools/webpack/packages.js- Using Gutenberg's script buildstools/webpack/script-modules.js- Using Gutenberg's module buildstools/webpack/development.js- Using Gutenberg's dev buildstools/webpack/vendors.js- Using Gutenberg's vendorstools/webpack/media.js- Kept (Core-specific files). to be honest, I'm not really sure if Webpack is needed for these files but I kept it unchanged for now, we could decide to remove the webpack dependency later entirely.Font Library Integration
Added
/wp-admin/font-library.phpas the first proof-of-concept using this new architecture.Some other changes.
src/wp-includes/script-modules.php@wordpress/interactivity(this has been removed from Gutenberg)Gruntfile.jspackage.jsonwp/*release branches in Gutenberg.Testing
I think there are a lot of simplifications we could do later (specially to the copy script) by aligning the folder structures more closely together (the build folder of Gutenberg, and the file structure for built scripts, styles, modules, pages... on Core). It does require changing some paths on Core. I do think it would be for the better but I kept it out of this PR.