Skip to content
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

Update dependencies for Twenty Twenty-One. #5672

Closed
wants to merge 5 commits into from

Conversation

desrosj
Copy link
Contributor

@desrosj desrosj commented Nov 14, 2023

This updates all Twenty Twenty-One dependencies to their latest versions and run npm audit fix.

The one exception to this is stylelint, which has only been updated to 14.16.1, the latest version of that major branch. The stylelint related dependencies maintained upstream in WordPress/gutenberg do not yet seem to support 15.x.

This update is primarily to get TT1 to a state where there are no errors when running install and build tasks on Node.js 18.x. Running npm install with 18.x currently produces the following output:

npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@es-joy/[email protected]',
npm WARN EBADENGINE   required: { node: '^12 || ^14 || ^16' },
npm WARN EBADENGINE   current: { node: 'v18.18.2', npm: '9.8.1' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '[email protected]',
npm WARN EBADENGINE   required: { node: '^12 || ^14 || ^16' },
npm WARN EBADENGINE   current: { node: 'v18.18.2', npm: '9.8.1' }
npm WARN EBADENGINE }

These updates fix this issue.

Trac tickets:


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@desrosj desrosj marked this pull request as ready for review November 14, 2023 18:53
@desrosj
Copy link
Contributor Author

desrosj commented Nov 14, 2023

@carolinan Do you mind giving this a once over? Mainly looking to get validation that these consolidated selectors will not be problematic.

The following errors are happening now when running build. This is likely due to that consolidated.

assets/css/ie-editor.css
 1237:43  ✖  Unexpected empty block  block-no-empty

assets/css/ie.css
 2665:43  ✖  Unexpected empty block  block-no-empty
 4135:43  ✖  Unexpected empty block  block-no-empty

@aaronjorbin
Copy link
Member

One option to fix this is to use postcss-discard-empty, here is a patch that I tested which does so.

diff --git a/src/wp-content/themes/twentytwentyone/package.json b/src/wp-content/themes/twentytwentyone/package.json
index 6681a5defe..83b02a0bae 100644
--- a/src/wp-content/themes/twentytwentyone/package.json
+++ b/src/wp-content/themes/twentytwentyone/package.json
@@ -27,6 +27,7 @@
 		"postcss-css-variables": "^0.19.0",
 		"postcss-custom-media": "^10.0.2",
 		"postcss-discard-duplicates": "^6.0.0",
+		"postcss-discard-empty": "^6.0.0",
 		"postcss-focus-within": "^8.0.0",
 		"postcss-merge-rules": "^6.0.1",
 		"postcss-nested": "^6.0.1",
diff --git a/src/wp-content/themes/twentytwentyone/postcss.config.js b/src/wp-content/themes/twentytwentyone/postcss.config.js
index d889f314ba..3f674aee71 100644
--- a/src/wp-content/themes/twentytwentyone/postcss.config.js
+++ b/src/wp-content/themes/twentytwentyone/postcss.config.js
@@ -9,6 +9,7 @@ module.exports = {
 			precision: 0
 		}),
 		require('postcss-discard-duplicates'),
-		require('postcss-merge-rules')
+		require('postcss-merge-rules'),
+		require('postcss-discard-empty')
 	]
 };

@carolinan
Copy link
Contributor

carolinan commented Nov 15, 2023

To get the correct line numbers for the errors, one can use npm run build:stylelint
The empty block in all 3 reported errors, is

@media only screen and (max-width: 481px) {
}

I have confirmed that the styles are indeed still present in the files, consolidated, moved to a different media query. I would expect the empty block to be removed, but this does not seem to happen for the media queries.
If it can't be removed with the help of the tools, I support Aarons suggestion to discard the empty blocks.

@desrosj
Copy link
Contributor Author

desrosj commented Nov 15, 2023

Thanks all! I went with @aaronjorbin's patch above.

@desrosj
Copy link
Contributor Author

desrosj commented Nov 17, 2023

Merged in core.trac.wordpress.org/changeset/57122.

@desrosj desrosj closed this Nov 17, 2023
@desrosj desrosj deleted the update/tt1-dependencies branch November 17, 2023 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants