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

Consider removing the app.js file #2027

Closed
Tracked by #1499
caendesilva opened this issue Nov 16, 2024 · 2 comments
Closed
Tracked by #1499

Consider removing the app.js file #2027

caendesilva opened this issue Nov 16, 2024 · 2 comments

Comments

@caendesilva
Copy link
Member

caendesilva commented Nov 16, 2024

Do we actually need the app.js file? We just use Alpine at the moment, so it's empty by default. The app.css file at least loads Tailwind.

@caendesilva
Copy link
Member Author

Claude Sonnet

Based on the provided code snippets, it appears that the app.js file is not strictly necessary in the current HydePHP setup. Here's my analysis:

  1. The app.js file is essentially empty, containing only a comment:
/*
* This is the main JavaScript used by Vite to build the app.js file.
*/
  1. The main JavaScript functionality currently comes from Alpine.js, which is loaded directly via CDN in the scripts layout:
{{-- Alpine.js --}}
<script defer src="https://cdn.jsdelivr.net/npm/alpinejs@3.10.3/dist/cdn.min.js" integrity="sha256-gOkV4d9/FmMNEkjOzVlyM2eNAWSUXisT+1RbMTTIgXI=" crossorigin="anonymous"></script>
  1. The Vite configuration does include app.js in its build process:
            input: [
                resolve(__dirname, 'resources/assets/app.js'),
                resolve(__dirname, 'resources/assets/app.css')
            ],
  1. However, looking at the test files, the only test for app.js just checks that it exists and contains a comment:
    public function testLaravelMixResourcesArePresent()
    {
        $this->assertFileExists(Hyde::path('resources/assets/app.css'));
        $this->assertFileExists(Hyde::path('resources/assets/app.js'));

        $this->assertFileContainsString('@tailwind base;', Hyde::path('resources/assets/app.css'));
        $this->assertFileContainsString('@tailwind components;', Hyde::path('resources/assets/app.css'));
        $this->assertFileContainsString('@tailwind utilities;', Hyde::path('resources/assets/app.css'));

        $this->assertFileContainsString('This is the main JavaScript', Hyde::path('resources/assets/app.js'));
    }

Recommendation:

  1. The app.js file could be safely removed since:

    • It's currently empty except for a comment
    • All JavaScript functionality is handled by Alpine.js via CDN
    • No other components seem to depend on it
    • The file is only included in development when Vite is running
  2. To remove it:

    • Delete resources/assets/app.js
    • Update the Vite config to only handle CSS
    • Update any tests that check for its existence
    • Update documentation to reflect that JavaScript is handled via Alpine.js CDN

However, you might want to keep the empty file as a convenience for users who want to add their own JavaScript in the future, since the build pipeline is already set up to handle it. This follows Hyde's philosophy of being ready for customization while not requiring it.

@caendesilva
Copy link
Member Author

Okay, in #2028 we will keep the app.js in resources to keep it easily available, but we won't compile the empty file. So that resolves the annoyance here.

@caendesilva caendesilva closed this as not planned Won't fix, can't repro, duplicate, stale Nov 16, 2024
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

No branches or pull requests

1 participant