Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a page-specific widget registration action, an early-return count path in site retrieval that sums domain counts, exposes bulk actions via a filter, and introduces ESLint/Stylelint configs plus npm scripts for multi-tool linting. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
inc/admin-pages/class-base-admin-page.phpinc/functions/site.phpinc/list-tables/class-site-list-table.php
🧰 Additional context used
🧬 Code graph analysis (1)
inc/functions/site.php (1)
inc/models/class-site.php (1)
Site(26-1942)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: PHP 8.1
- GitHub Check: PHP 8.0
- GitHub Check: Build Plugin for Testing
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: cypress (8.2, chrome)
🔇 Additional comments (3)
inc/list-tables/class-site-list-table.php (1)
380-388: LGTM! Good extensibility addition.The filter
wu_site_list_get_bulk_actionsis well-documented and allows external modification of bulk actions, improving extensibility for plugins and themes.inc/admin-pages/class-base-admin-page.php (2)
626-626: LGTM! New hook provides extensibility for widget registration.The new action hook at priority 21 enables addons to register widgets after the core registration completes.
734-756: LGTM! Well-documented extensibility method.The
fire_register_widgets_hook()method is properly documented and provides a clean extension point for page-specific widget registration via the dynamic actionwu_page_{$this->id}_register_widgets.
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.stylelintrc.js (1)
1-36: Consider using 'warn' instead of disabling all rules.Disabling every Stylelint rule (setting to
null) negates the value of introducing the linter. Instead of completely disabling rules, consider:
- Setting rules to
'warn'initially to identify issues without blocking CI- Gradually enabling rules as
'error'after addressing violations- Using more targeted overrides for specific legacy files
This approach provides visibility into code quality issues while allowing incremental improvement.
🔎 Example: Convert disabled rules to warnings
module.exports = { extends: ['@wordpress/stylelint-config'], rules: { // Allow !important for WordPress admin overrides - 'declaration-no-important': null, + 'declaration-no-important': 'warn', // Relax selector specificity for admin styling - 'selector-max-id': null, + 'selector-max-id': 'warn', // Allow vendor prefixes (handled by build tools if needed) - 'property-no-vendor-prefix': null, - 'value-no-vendor-prefix': null, + 'property-no-vendor-prefix': 'warn', + 'value-no-vendor-prefix': 'warn', // ... apply same pattern to remaining rules },
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
.eslintrc.js.stylelintrc.jspackage.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: Build Plugin for Testing
🔇 Additional comments (6)
.stylelintrc.js (1)
37-43: LGTM!The ignore patterns appropriately exclude minified files, third-party code, and vendor directories from linting.
package.json (2)
6-11: LGTM!The authors field normalization to structured object format follows npm package.json conventions.
52-59: LGTM! Lint workflow is well-structured.The multi-tool linting approach with separate commands for PHP, JavaScript, and CSS is clean and maintainable. Using
run-pfor parallel execution is efficient, and the ignore patterns for minified files are appropriate.Note: These scripts depend on the
.eslintrc.jsand.stylelintrc.jsconfiguration files added in this PR..eslintrc.js (3)
1-7: LGTM!The base ESLint configuration correctly sets
root: true, extends the WordPress ESLint plugin, and declares appropriate environments for a WordPress plugin (browser and jQuery).
65-71: LGTM!The ignore patterns appropriately exclude minified JavaScript files, third-party code, and vendor directories from linting.
8-35: The declared globals are actively used in the codebase.Review of the actual JavaScript files confirms that the globals declared in
.eslintrc.jsare used throughout the codebase. Examples include:
wu_on_load()called in admin.js, list-tables.js, and vue-apps.jswu_settingsreferenced extensively in functions.js and dashboard-statistics.jswu_format_money(),wu_block_ui(), and otherwu_*functions defined and usedwpglobal used via hooks, editor, media, and codeEditor APIs- Third-party globals like
ClipboardJS,Vue, andSwalused in application codewuboxreferenced across multiple modulesThe extensive globals list does suggest tight coupling and reliance on window-scoped state, which may warrant future refactoring toward more modular patterns. However, all declared globals serve active purposes in the current codebase.
Likely an incorrect or invalid review comment.
| rules: { | ||
| // Allow tabs for indentation (matches PHP coding standards) | ||
| indent: ['error', 'tab', { SwitchCase: 1 }], | ||
| // Disable prettier - too strict for legacy code | ||
| 'prettier/prettier': 'off', | ||
| // Relax some rules for legacy code compatibility | ||
| 'no-unused-vars': ['warn', { vars: 'all', args: 'none', ignoreRestSiblings: true }], | ||
| // Allow console for development | ||
| 'no-console': 'warn', | ||
| // Allow var for legacy code | ||
| 'no-var': 'warn', | ||
| // Prefer const but don't enforce strictly for legacy code | ||
| 'prefer-const': 'warn', | ||
| // Object shorthand is nice but not required for legacy code | ||
| 'object-shorthand': 'warn', | ||
| // Allow snake_case for WP compatibility | ||
| camelcase: 'off', | ||
| // Allow redeclaring globals (we define them above) | ||
| 'no-redeclare': 'off', | ||
| // Disable strict formatting rules for legacy code | ||
| 'space-in-parens': 'off', | ||
| 'comma-dangle': 'off', | ||
| quotes: 'off', | ||
| semi: 'off', | ||
| 'padded-blocks': 'off', | ||
| 'eol-last': 'off', | ||
| 'space-before-function-paren': 'off', | ||
| 'space-before-blocks': 'off', | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Reconsider disabling critical rules like 'no-redeclare'.
While relaxing formatting rules for legacy code is understandable, disabling certain rules can introduce bugs:
-
Line 54 (
no-redeclare: 'off'): Disabling this rule allows redeclaring variables, which can cause subtle bugs. The comment suggests it's disabled to allow redeclaring the globals from lines 8-35, but this shouldn't be necessary if globals are properly declared. -
Line 40 (
prettier/prettier: 'off'): If you're not using Prettier, consider removing it from the extends chain in line 3 (recommended-with-formatting) to avoid loading unused rules. -
Formatting rules (lines 56-63): Consider setting these to
'warn'instead of'off'to maintain visibility into code style issues.
🔎 Recommended changes
rules: {
// Allow tabs for indentation (matches PHP coding standards)
indent: ['error', 'tab', { SwitchCase: 1 }],
// Disable prettier - too strict for legacy code
'prettier/prettier': 'off',
// Relax some rules for legacy code compatibility
'no-unused-vars': ['warn', { vars: 'all', args: 'none', ignoreRestSiblings: true }],
// Allow console for development
'no-console': 'warn',
// Allow var for legacy code
'no-var': 'warn',
// Prefer const but don't enforce strictly for legacy code
'prefer-const': 'warn',
// Object shorthand is nice but not required for legacy code
'object-shorthand': 'warn',
// Allow snake_case for WP compatibility
camelcase: 'off',
- // Allow redeclaring globals (we define them above)
- 'no-redeclare': 'off',
+ // Warn on redeclarations instead of allowing them
+ 'no-redeclare': 'warn',
// Disable strict formatting rules for legacy code
- 'space-in-parens': 'off',
- 'comma-dangle': 'off',
- quotes: 'off',
- semi: 'off',
- 'padded-blocks': 'off',
- 'eol-last': 'off',
- 'space-before-function-paren': 'off',
- 'space-before-blocks': 'off',
+ 'space-in-parens': 'warn',
+ 'comma-dangle': 'warn',
+ quotes: 'warn',
+ semi: 'warn',
+ 'padded-blocks': 'warn',
+ 'eol-last': 'warn',
+ 'space-before-function-paren': 'warn',
+ 'space-before-blocks': 'warn',
},🤖 Prompt for AI Agents
.eslintrc.js lines 36-64: the config currently disables critical rules
(no-redeclare, prettier/prettier, and several formatting rules) which can mask
bugs and unused config; re-enable 'no-redeclare' (set to 'error' or at least
'warn') and instead ensure globals are declared in the top-level "globals"
section or via /* global */ comments so redeclaration is not needed; if Prettier
is not used, remove 'recommended-with-formatting' from extends and re-enable the
'prettier/prettier' rule (or set to 'warn') rather than turning it off; change
the formatting rules currently set to 'off' (space-in-parens, comma-dangle,
quotes, semi, padded-blocks, eol-last, space-before-function-paren,
space-before-blocks) to 'warn' to surface style issues while maintaining legacy
flexibility.
| "@eslint/js": "^9.39.2", | ||
| "@wordpress/env": "^10.27.0", | ||
| "@wordpress/eslint-plugin": "^23.0.0", | ||
| "@wordpress/stylelint-config": "^23.29.0", | ||
| "clean-css-cli": "^5.6.3", | ||
| "cypress": "^14.5.4", | ||
| "cypress-mailpit": "^1.4.0", | ||
| "eslint": "^8.57.1", | ||
| "globals": "^16.5.0", | ||
| "npm-run-all": "^4.1.5", | ||
| "stylelint": "^16.26.1", | ||
| "stylelint-config-standard": "^39.0.1", | ||
| "uglify-js": "^3.19.3" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check npm registry for latest versions
echo "=== Latest versions on npm ==="
npm view eslint@latest version 2>/dev/null || echo "eslint latest check failed"
npm view @eslint/js@latest version 2>/dev/null || echo "@eslint/js latest check failed"
npm view @wordpress/eslint-plugin@latest version 2>/dev/null || echo "@wordpress/eslint-plugin latest"
npm view @wordpress/stylelint-config@latest version 2>/dev/null || echo "@wordpress/stylelint-config latest"
npm view stylelint@latest version 2>/dev/null || echo "stylelint latest check failed"
npm view stylelint-config-standard@latest version 2>/dev/null || echo "stylelint-config-standard latest"
npm view globals@latest version 2>/dev/null || echo "globals latest"
npm view cypress@latest version 2>/dev/null || echo "cypress latest"
npm view clean-css-cli@latest version 2>/dev/null || echo "clean-css-cli latest"
npm view npm-run-all@latest version 2>/dev/null || echo "npm-run-all latest"
npm view uglify-js@latest version 2>/dev/null || echo "uglify-js latest"
echo -e "\n=== Package peer dependencies ==="
npm view @eslint/js@9.39.2 peerDependencies 2>/dev/null || echo "Could not check @eslint/js peerDependencies"
npm view eslint@8.57.1 version 2>/dev/null || echo "Could not verify eslint@8.57.1 exists"
npm view eslint@9 version 2>/dev/null || echo "eslint v9 check"
echo -e "\n=== Check for vulnerabilities in eslint and related packages ==="
npm audit --json 2>/dev/null | python3 -c "import sys, json; data = json.load(sys.stdin); vulns = data.get('vulnerabilities', {}); print('Total vulnerabilities found:', len(vulns)); [print(f'{pkg}: {v}') for pkg, v in list(vulns.items())[:10]]" 2>/dev/null || echo "No npm audit data available"Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 167
🌐 Web query:
ESLint 8.x end of life date October 2024
💡 Result:
Yes — ESLint v8.x reached end of life on October 5, 2024. [1][2]
Sources:
- ESLint announcement: "ESLint v8.x end of life is October 5, 2024." [1]
- ESLint version support page (policy explaining EOL timing). [2]
🌐 Web query:
@eslint/js 9.x peer dependency requirements eslint version
💡 Result:
@eslint/js 9.x does not declare a peerDependency on eslint (the package shows 0 dependencies / no peerDependencies). [1][2]
Sources:
- @eslint/js on npm (v9.29.0). [1]
- @eslint/js on npm (v9.9.1). [2]
Upgrade ESLint to version 9.x
ESLint 8.x reached end of life on October 5, 2024. Consider upgrading eslint to version 9.x for continued support and security updates.
Note: The presence of @eslint/js@^9.39.2 alongside eslint@^8.57.1 does not cause compatibility issues—@eslint/js has no peer dependency on the eslint package and can work independently. However, upgrading ESLint itself is still recommended for maintenance and support purposes.
🤖 Prompt for AI Agents
In package.json around lines 17 to 29, eslint is pinned to ^8.57.1 while
@eslint/js is already 9.x; update the eslint dependency to a 9.x release (e.g.
^9.##) in package.json, run npm/yarn install to update the lockfile, run the
lint suite and CI to catch any rule/config incompatibilities, and if tests or
linters surface breaking changes, adjust ESLint config/plugins (parser options,
rules, or plugin versions) accordingly before committing the lockfile changes.
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
1 participant
Summary by CodeRabbit
New Features
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.