rappasoft / laravel-livewire-tables

A dynamic table component for Laravel Livewire
https://rappasoft.com/docs/laravel-livewire-tables/v2/introduction
MIT License
1.78k stars 335 forks source link

[Bug]: When using asset injection in some environments, core.min.js is missing closing ); #1423

Closed nathan-io closed 1 year ago

nathan-io commented 1 year ago

What happened?

In our app, we recently upgraded Laravel from 9 to 10, Livewire from 2 to 3, and livewire-tables to from 2.15.0 to v3.0-beta.3.

On my local (Ubuntu 20 on WSL2), our livewire-tables tables seem to be working fine.

However, on our staging instance (a Forge-provisioned server), as well as on one of our developer's local instances, we're seeing strange behavior:

On all pages, there are two new console errors:

core.min.js:3 Uncaught SyntaxError: missing ) after argument list (at core.min.js:3:6392)
livewire.js?id=178de384:7707 Uncaught SyntaxError: Invalid or unexpected token (at livewire.js?id=178de384:7707:1)

On pages which contains a livewire-table, there are additional console errors.

Visually, two things are happening:


I copied core.min.js from both instances. Diff here.

You can see the missing ); in the version from staging. The version from my local has it.

This core.min.js response seems to come from vendor/rappasoft/laravel-livewire-tables/resources/js/laravel-livewire-tables.min.js.

When I look at that file, the missing ); is there.

On our staging instance, I've tried:

  1. rm -rf vendor && composer install
  2. rm -rf node_modules && npm i && npm run build
  3. php artisan view:clear && php artisan optimize:clear && php artisan clear
  4. Disabled Cloudflare cache
  5. Restarted the entire server

I didn't think #3 was necessary, just wanted to be thorough.

I've also confirmed that both my local and staging have the same version of laravel/framework, livewire/livewire, and rappasoft/livewire-tables.

Despite this, the core.min.js on staging remains the same, even when I access the URL directly and do a hard refresh.

As noted above, this is also happening on the local instance of our developer.

How to reproduce the bug

Unsure how to reproduce.

Package Version

v3.0.0-beta.3

PHP Version

8.1.x

Laravel Version

10.28.0

Alpine Version

3.12.0

Theme

Tailwind 3.x

Notes

It isn't something related to a particular branch. I've tried different branches on my local, and everything works as expected.

Our Forge server is running Node 20.8.1 and npm 10.1.0. On my local, where everything works, Node is 18.2.0 and npm is v9.7.1. So I don't think it's related to the npm version.

Error Message

No response

nathan-io commented 1 year ago

As a follow up, our developer that is having this same issue on his machine is using npm 9.8.1 and node 18.18.0.

I did a fresh build on my laptop and everything works fine there (same as on my work machine), using npm 9.6.7 and node 18.2.0.

I suppose this really isn't an issue with the package itself, but I'm not very experienced in this area and was just hoping that someone may have an insight as to why this is happening only on some instances of our app.

nathan-io commented 1 year ago

After realizing that core.min.js was being injected and looking into how that works, I think the issue lies with the output of the response()->file() in pretendResponseIsJs().

We had an issue with this elsewhere in our app, when trying to return a PDF file as a response. The response was malformed. The solve for that was to add ob_end_clean().

$response = response()->file($file);
ob_end_clean();

I got that solution from https://laracasts.com/discuss/channels/laravel/image-is-being-thrown-as-a-white-blank-image?reply=132045

It's possible that response()->file() has some quirks depending on the environment.

I've confirmed that even in the instances/environments where we were having the issue, vendor/rappasoft/laravel-livewire-tables/resources/js/laravel-livewire-tables.min.js is valid JS and includes the missing );. So the source file is ok, but it seems that the ); is getting chopped off in some environments by response->file().


We were able to bypass the issue by using bundler including, though we weren't able to get it working as documented here with Vite due to a postcss error.

Rather, we're importing like so:

common.js:

import '../../vendor/rappasoft/laravel-livewire-tables/resources/js/laravel-livewire-tables.min.js';

common.css:

@import '../../vendor/rappasoft/laravel-livewire-tables/resources/css/laravel-livewire-tables.min.css';

I'm trying to get more details on the error that occurred when we used just a single import '../../vendor/rappasoft/laravel-livewire-tables/resources/imports/laravel-livewire-tables-all.js'; in our common.js.

lrljoe commented 1 year ago

I'll take a look this evening, and will be around in the Rappasoft Discord

We reuse the livewire core injection capability, so it should just work.

Will have another look, which precise version of the package are you on beta1,beta2,beta3, or one of the dev branches?

nathan-io commented 1 year ago

Sounds good. We're on beta3.

lrljoe commented 1 year ago

Cheers.

It'll definitely be working with bundling via Vite, as that's what I have in my production env

Just need to see if anything was broken in the interim!

nathan-io commented 1 year ago

The error thrown by npm when we use import '../../vendor/rappasoft/laravel-livewire-tables/resources/imports/laravel-livewire-tables-all.js'; in our common.js:

[vite:css] [postcss] Cannot read properties of undefined (reading 'config')
file: /path/to/project/vendor/rappasoft/laravel-livewire-tables/resources/css/laravel-livewire-tables-thirdparty.min.css:undefined:undefined
error during build:
TypeError: [postcss] Cannot read properties of undefined (reading 'config')
    at getTailwindConfig (/path/to/project/node_modules/tailwindcss/lib/lib/setupTrackingContext.js:85:63)
    at /path/to/project/node_modules/tailwindcss/lib/lib/setupTrackingContext.js:97:92
    at /path/to/project/node_modules/tailwindcss/lib/processTailwindFeatures.js:46:11
    at plugins (/path/to/project/node_modules/tailwindcss/lib/plugin.js:38:63)
    at LazyResult.runOnRoot (/path/to/project/node_modules/postcss/lib/lazy-result.js:339:16)
    at LazyResult.runAsync (/path/to/project/node_modules/postcss/lib/lazy-result.js:393:26)
    at LazyResult.async (/path/to/project/node_modules/postcss/lib/lazy-result.js:221:30)
    at LazyResult.then (/path/to/project/node_modules/postcss/lib/lazy-result.js:206:17)
nathan-io commented 1 year ago

Just keep in mind that it worked fine with injection on both of my local machines on beta3, running Ubuntu on WSL2.

We have the issue on our remote server and on one of our developer's machines, both of which are running native Ubuntu. I think this is what causes the difference in the output of response()->file().

lrljoe commented 1 year ago

I've got a native Ubuntu instance, and it doesn't get upset, when using a bundler approach, with:

import '../../vendor/rappasoft/laravel-livewire-tables/resources/imports/laravel-livewire-tables-all.js';

in my "app.js" (which is the root for my bundler)

I'll see if I can replicate your error on my end though!

I'm wondering if there's a library that I'm using, or something you're using, that is trying to resolve something that doesn't exist.

Are you using flatpickr elsewhere in your project out of interest?

It won't be the response()->file() bit with NPM, as that's only used for the injection method to "spoof" the file (as it's not accessible publicly from your vendor folder)

nathan-io commented 1 year ago

Are you using flatpickr elsewhere in your project out of interest?

I don't believe so.

Our vite.config.js:

import { defineConfig } from 'vite';
import laravel, { refreshPaths } from 'laravel-vite-plugin';

export default defineConfig({
    plugins: [
        laravel({
            input: [
                'resources/css/app.css',
                'resources/js/app.js',
                'resources/css/site.css',
                'resources/js/site.js',
            ],
            refresh: [
                ...refreshPaths,
                'app/Http/Livewire/**',
            ],
        }),
    ],
    resolve: {
        alias: {
            '@node': '/node_modules',
        },
    },
});

package.json

{
    "private": true,
    "scripts": {
        "dev": "vite",
        "build": "vite build"
    },
    "devDependencies": {
        "@alpinejs/focus": "^3.10.5",
        "@tailwindcss/forms": "^0.5.2",
        "@tailwindcss/typography": "^0.5.0",
        "alpinejs": "^3.12",
        "autoprefixer": "^10.4.7",
        "axios": "^0.27",
        "laravel-echo": "^1.15.2",
        "laravel-vite-plugin": "^0.8.0",
        "pusher-js": "^8.3.0",
        "tailwindcss": "^3.2.6",
        "vite": "^4.1.1",
        "postcss": "^8.4.14"
    },
    "dependencies": {
        "@alpinejs/collapse": "^3.12.0",
        "@alpinejs/focus": "^3.12.0",
        "@alpinejs/mask": "^3.12.0",
        "@alpinejs/ui": "^3.12.0-beta.0",
        "@caneara/iodine": "^8.3.0",
        "@splidejs/splide": "^4.1.4",
        "alpinejs": "^3.12.0",
        "intl-tel-input": "^17.0.21",
        "tippy.js": "^6.3.7"
    }
}
nathan-io commented 1 year ago

With regard to the injection issue, is core.min.js missing the closing ); when using injection on your native Ubuntu machine?

lrljoe commented 1 year ago

I don't get that error appearing in any of my test/dev envs (which are all either Ubuntu or Debian), I'll try a fresh install and see if anything complains at that point.

lrljoe commented 1 year ago

Just ran some tests, and unable to replicate at the moment.

If you could please share initially your vite.config.js, I'll then try to replicate the issue on my end.

I'm sure you're probably doing something cleverer than I am, so it'd be good to try to replicate it properly.

nathan-io commented 1 year ago

Thanks but it's probably the other way around! It's in one of the comments above.

lrljoe commented 1 year ago

Ah k, spotted that vite config.

Couple of quick questions for you here.

Vite/Laravel both advise that you don't add CSS files as inputs, instead add them as imports into your JS. It still separates them out properly.

E.g.

    plugins: [
        laravel({
            input: [
                'resources/js/app.js'
            ],

Then my app.js has:

import "../css/app.css";

Now I lost track of why exactly they recommend this, but I know it works okay for me!

Next up, you seem to be missing the common alias, that Laravel advise you add:

    resolve: {
        alias: {
            '@': '/resources/js'
        }
    }

Finally, I don't see any postcss etc going on there, so how are you building your Tailwind etc (minimally)

lrljoe commented 1 year ago

As an example of a vite config that I use for one of my demo environments. This configures a minimised Tailwind CSS, has some minify options (some of which aren't used), resolves common js libraries that it might need etc.

import { defineConfig } from 'vite';
import laravel from 'laravel-vite-plugin';
import tailwindcss from 'tailwindcss';
import autoprefixer from 'autoprefixer';
import cssnano from 'cssnano';
import resolve from '@rollup/plugin-node-resolve';
import commonjs from '@rollup/plugin-commonjs';

export default defineConfig({
    build: {
        rollupOptions: {
            plugins: [
                resolve(),
                commonjs(),
            ]
        }
    },
    plugins: [
        laravel({
            input: [
                'resources/js/app.js'
            ],
            refresh: true,
            postcss: [
                tailwindcss(),
                autoprefixer(),
                cssnano({
                    preset: 'default',
                }),
            ],
        })
    ],
    resolve: {
        alias: {
            '@': '/resources/js'
        }
    }

});

With app.js containing:

import './bootstrap';
import { Livewire, Alpine } from '../../vendor/livewire/livewire/dist/livewire.esm';
import Clipboard from '@ryangjchandler/alpine-clipboard'
import focus from '@alpinejs/focus';
import '../../vendor/rappasoft/laravel-livewire-tables/resources/imports/laravel-livewire-tables-all';
import "../css/app.css";

Alpine.plugin(Clipboard)
Alpine.plugin(focus)
Livewire.start()

What does your app.js and site.js look like out of interest?

nathan-io commented 1 year ago

Thanks and sorry for the delay getting back to you!

Our original frontend dev recently moved on from the project but I'll reach out to see why it was done like that. I want to make sure we have a proper setup.

Vite/Laravel both advise that you don't add CSS files as inputs, instead add them as imports into your JS.

I think I see what you're talking about in the Laravel docs:

If you are building an SPA, including applications built using Inertia, Vite works best without CSS entry points:

Our app isn't really an SPA and we aren't using Interia, but maybe this is also a best practice for apps using Livewire?

As for the Vite docs, I couldn't find anything about this but this is admittedly not really my area.

Next up, you seem to be missing the common alias, that Laravel advise you add:

To my understanding this alias is defined by Laravel by default.

I don't see any postcss etc going on there, so how are you building your Tailwind etc (minimally)

I think this is working because we have a postcss.config.js matching the docs here:

module.exports = {
    plugins: {
        tailwindcss: {},
        autoprefixer: {},
    },
};

I've noticed that when I add a previously unused Tailwind class in a view, I have to rebuild assets before it is included.

lrljoe commented 1 year ago

@nathan-io - yep, that's how the tailwindcss/autoprefix works, it only extracts the classes you are using.

Using tailwind.config.js you can tell it specific classes to always add, or content to include from 3rd parties using purge.content and purge.safelist

lrljoe commented 1 year ago

@nathan-io - are you still experiencing this issue? If so, please respond to re-open the issue.

nathan-io commented 1 year ago

As to the original issue, I don't know because we switched from injection to bundling in order to bypass it. I think this is something that others could see as well as response()->file() doesn't seem to behave consistently across environments.

https://github.com/rappasoft/laravel-livewire-tables/issues/1423#issuecomment-1766668468

The issue where we had to tweak the asset bundling could be related to the specifics of our Vite setup, I'm not sure.