Skip to content

Conversation

superdav42
Copy link
Collaborator

@superdav42 superdav42 commented Sep 20, 2025

Summary

  • Replace assets/js/lib/vue.min.js with the official Vue 2.7.14 production build (no dev warnings) and append a wrapper to expose window.wu_vue = { Vue: window.Vue, defineComponent: window.Vue && window.Vue.defineComponent ? window.Vue.defineComponent : (x)=>x }.
  • Keep development build as-is; production environments load the minified production build via wu_get_asset() when SCRIPT_DEBUG is false.
  • Add PHPUnit tests to verify production flags (productionTip:!1, devtools:!1) and the presence of the window.wu_vue wrapper.

Rationale

  • Addresses Vue js is in development mode #24 (“Vue js is in development mode”). Previous vue.min.js was a minified dev build (dev warnings enabled) and lacked the wu_vue export seen in the dev file. This change standardizes production behavior and preserves compatibility with existing scripts that reference window.wu_vue.

Testing

  • vendor/bin/phpunit passes (added tests/unit/VueBuild_Test.php).
  • Manual grep confirms production flags and wrapper present in assets/js/lib/vue.min.js.

Notes

  • The wrapper uses Vue.defineComponent when available on the global build; otherwise falls back to a pass-through function, matching existing usage expectations.

Closes #24

Summary by CodeRabbit

  • Tests
    • Added automated checks to verify the production Vue bundle is present, uses production-safe settings, and exposes expected global APIs.
    • Strengthens CI by detecting build regressions early, improving reliability and stability of the app.
    • No user-facing behavior changes; enhances confidence in production builds.

Copy link
Contributor

coderabbitai bot commented Sep 20, 2025

Walkthrough

Introduces a PHPUnit test class to validate the Vue.js production build file. The tests assert the minified file exists and contains production flags, a global wrapper, and exposed API via the wrapper by scanning assets/js/lib/vue.min.js for specific strings and a regex.

Changes

Cohort / File(s) Summary of changes
Unit tests for Vue production build
tests/unit/VueBuild_Test.php
Added PHPUnit tests verifying: file presence/readability of assets/js/lib/vue.min.js; production flags (productionTip:false, devtools:false); presence of window.wu_vue wrapper; exposure of defineComponent through wrapper via regex/string checks.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I twitch my ears at minified light,
Sniffing flags that hide the dev-night;
A wrapper named wu_vue in sight—
defineComponent hops just right.
Tests nibble bytes with careful cheer,
Production sure, the path is clear.
Thump-thump: release draws near! 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Linked Issues Check ❓ Inconclusive The PR demonstrates replacement of the dev build with a Vue 2.7.14 production build, adds the window.wu_vue wrapper, and introduces PHPUnit tests that assert production flags and wrapper presence, which addresses the core linked-issue goals of removing dev-mode warnings and exposing the wrapper API. However, the linked issue specifically required preserving any project-specific build modifications present in the previous build, and the provided summary and tests do not show a diff or explicit verification that those bespoke modifications were retained, so compliance with that particular objective cannot be confirmed. Please provide the diff showing how project-specific build modifications were preserved (or confirm none existed) and/or add tests or a compatibility checklist that demonstrate any previously relied-on build tweaks remain functional.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title succinctly describes the primary change — switching to the Vue 2.7.14 production build and exposing a window.wu_vue wrapper — and directly matches the files and tests summarized in the changeset, making it clear and relevant for reviewers scanning history.
Out of Scope Changes Check ✅ Passed All changes described (replacement of assets/js/lib/vue.min.js with a production build, appending the wu_vue wrapper, and adding tests under tests/unit) are directly related to the linked issue objectives and PR description, and there is no evidence in the provided summary of unrelated or extraneous modifications.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@superdav42 superdav42 added this to the 2.4.5 milestone Sep 20, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (4)
tests/unit/VueBuild_Test.php (4)

5-6: DRY the path and file reads; add a small helper.

Reduces duplication and improves failure messages consistently across tests.

 final class VueBuild_Test extends TestCase {
+    private const VUE_MIN_PATH = __DIR__ . '/../../assets/js/lib/vue.min.js';
+
+    private function readVueMin(): string {
+        $this->assertFileExists(self::VUE_MIN_PATH, 'vue.min.js does not exist');
+        $contents = file_get_contents(self::VUE_MIN_PATH);
+        $this->assertIsString($contents);
+        return $contents;
+    }
     public function test_vue_min_is_production_flags(): void {
-        $path = __DIR__ . '/../../assets/js/lib/vue.min.js';
-        $this->assertFileExists($path, 'vue.min.js does not exist');
-        $contents = file_get_contents($path);
-        $this->assertNotFalse($contents);
+        $contents = $this->readVueMin();
         // Production build should set productionTip:false and devtools:false
         $this->assertStringContainsString('productionTip:!1', $contents, 'Expected productionTip disabled in production build');
         $this->assertStringContainsString('devtools:!1', $contents, 'Expected devtools disabled in production build');
     }
 
     public function test_vue_min_has_wu_vue_wrapper(): void {
-        $path = __DIR__ . '/../../assets/js/lib/vue.min.js';
-        $contents = file_get_contents($path);
-        $this->assertNotFalse($contents);
+        $contents = $this->readVueMin();
         $this->assertStringContainsString('window.wu_vue', $contents, 'Expected window.wu_vue wrapper present');
         $this->assertMatchesRegularExpression('/defineComponent/', $contents, 'Expected defineComponent to be exposed via wrapper');
     }
 }

Also applies to: 16-16


10-12: Make production-flag checks less brittle and assert version + no dev banner.

Minifiers and upstream changes can flip between “:=” forms or tweak spacing. Also assert the official version header and absence of the dev-mode banner.

-        // Production build should set productionTip:false and devtools:false
-        $this->assertStringContainsString('productionTip:!1', $contents, 'Expected productionTip disabled in production build');
-        $this->assertStringContainsString('devtools:!1', $contents, 'Expected devtools disabled in production build');
+        // Production build should set productionTip:false and devtools:false (allow ":" or "=")
+        $this->assertMatchesRegularExpression('/productionTip\s*[:=]\s*!1/', $contents, 'Expected productionTip disabled in production build');
+        $this->assertMatchesRegularExpression('/devtools\s*[:=]\s*!1/', $contents, 'Expected devtools disabled in production build');
+        // Sanity: dev-mode banner should not be present
+        $this->assertStringNotContainsString('You are running Vue in development mode', $contents, 'Dev-mode banner should be absent in production build');
+        // Optional: lock to the intended upstream version
+        $this->assertMatchesRegularExpression('/Vue\.js v2\.7\.14\b/i', $contents, 'Expected official Vue 2.7.14 header present');

19-20: Strengthen wrapper assertions to verify shape, not just substrings.

Current regex matches any “defineComponent” in the file. Tighten to the actual wrapper mapping with fallback.

-        $this->assertStringContainsString('window.wu_vue', $contents, 'Expected window.wu_vue wrapper present');
-        $this->assertMatchesRegularExpression('/defineComponent/', $contents, 'Expected defineComponent to be exposed via wrapper');
+        $this->assertMatchesRegularExpression('/window\.wu_vue\s*=\s*{/', $contents, 'Expected window.wu_vue wrapper assignment');
+        $this->assertMatchesRegularExpression('/Vue\s*:\s*window\.Vue/', $contents, 'Expected wrapper to expose Vue');
+        $this->assertMatchesRegularExpression('/defineComponent\s*:\s*window\.Vue\s*&&\s*window\.Vue\.defineComponent\s*\?\s*window\.Vue\.defineComponent\s*:\s*\(\w+\)\s*=>\s*\w+/', $contents, 'Expected defineComponent mapping with fallback');

1-1: Optional: enable strict types in tests.

Not required, but keeps assertions/types consistent.

-<?php
+<?php
+declare(strict_types=1);
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fb0746d and 832dbb7.

⛔ Files ignored due to path filters (1)
  • assets/js/lib/vue.min.js is excluded by !**/*.min.js
📒 Files selected for processing (1)
  • tests/unit/VueBuild_Test.php (1 hunks)
⏰ 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). (13)
  • GitHub Check: cypress (8.1, chrome)
  • GitHub Check: cypress (8.4, firefox)
  • GitHub Check: cypress (8.2, chrome)
  • GitHub Check: cypress (8.3, firefox)
  • GitHub Check: cypress (8.4, chrome)
  • GitHub Check: cypress (8.1, firefox)
  • GitHub Check: cypress (8.2, firefox)
  • GitHub Check: cypress (8.3, chrome)
  • GitHub Check: cypress (8.0, firefox)
  • GitHub Check: cypress (7.4, firefox)
  • GitHub Check: cypress (7.4, chrome)
  • GitHub Check: cypress (8.0, chrome)
  • GitHub Check: PHP 8.1
🔇 Additional comments (2)
tests/unit/VueBuild_Test.php (2)

4-4: Good addition: sanity tests for prod build and wrapper.

Covers the critical flags and wrapper exposure. Nice.


20-20: Confirmed — PHPUnit supports assertMatchesRegularExpression/assertIsString

composer.json requires ^9.6.22 and composer.lock is locked to phpunit 9.6.23, so those assertions are available; no changes required.

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.

Vue js is in development mode
1 participant