lepikhinb / laravel-typescript

MIT License
387 stars 46 forks source link

Cannot redeclare helpers when running php artisan typescript:generate #6

Closed SlyDave closed 3 years ago

SlyDave commented 3 years ago

Getting strange behaviour when running php artisan typescript:generate it appears it's trying to redeclare all the helpers which are auto loaded via composer.json's "autoload" { "files" : [ "app/helpers.php"] }

$ php artisan typescript:generate
PHP Fatal error:  Cannot redeclare selectedVesselId() (previously declared in C:\www\workrest\app\helpers.php:25) in C:\www\workrest\app\Core\helpers.php on line 23

   Symfony\Component\ErrorHandler\Error\FatalError 

  Cannot redeclare selectedVesselId() (previously declared in C:\www\workrest\app\Core\helpers.php:25)

This is only occurring when running typescript:generate, other CLI commands for artisan are working fine e.g.:

$ php artisan cache:clear
Application cache cleared!
SlyDave commented 3 years ago

if I remove the autoload (breaking my app) the command runs, but swifty encounters another error in a Factory class ?!

   ErrorException 

  Undefined variable $factory

  at C:\www\workrest\database\factories\ComplianceFactory.php:9
      5▕ use Illuminate\Support\Carbon;
      6▕ use App\Models\Compliance;
      7▕
      8▕ /** @var \Illuminate\Database\Eloquent\Factory $factory */
  ➜   9▕ $factory->define(
     10▕     Compliance::class,
     11▕     function (Faker $faker) {
     12▕         return [
     13▕             'short_name' => 'IMO MLC 2006',

  1   C:\www\workrest\database\factories\ComplianceFactory.php:9
      Illuminate\Foundation\Bootstrap\HandleExceptions::handleError("Undefined variable $factory", "C:\www\workrest\database\factories\ComplianceFactory.php")

  2   C:\www\workrest\vendor\composer\ClassLoader.php:480
      include("C:\www\workrest\database\factories\ComplianceFactory.php")
SlyDave commented 3 years ago

Ah that's because of the psr-4 namespacing in composer

    "autoload": {
        "files": [
            "app/Core/helpers.php"
        ],
        "psr-4": {
            "App\\": "app/",
            "WorkRest\\": "WorkRest/",
            "Tests\\": "tests/",
            "Database\\Factories\\": "database/factories/",
            "Database\\Seeders\\": "database/seeders/"
        }
    },

the generator isn't smart enough to work out what is and isn't a Modal?

I should note this is with the default configuration in config/typescript

return [
    'generators' => [
        Model::class => ModelGenerator::class,
    ],

    'output' => resource_path('js/models.d.ts'),

    'autoloadDev' => false,
];
SlyDave commented 3 years ago

If I rip out all the non Modal autoloads, do a composer update, then run the generate command, it does work... (tho this obviously isn't sustainable or desired as it breaks all sorts of other things :D)

Seems like the generator needs to check the class it's loading is actual extending Illuminate\Database\Eloquent\Model ? rather than assuming everything in autoload is a Model..?

lepikhinb commented 3 years ago

Can I see your helpers.php file? Also, is that Laravel 8? The factories are actual classes since 7(8?).

SlyDave commented 3 years ago

I'd like to further add, that out of the box Laravel 7 and 8 have the following in the composer.json:

    "autoload": {
        "psr-4": {
            "App\\": "app/",
            "Database\\Factories\\": "database/factories/",
            "Database\\Seeders\\": "database/seeders/"
        }
    },

so on a Vanilla install, the generate command is likely to fail... :(

lepikhinb commented 3 years ago

I'd like to further add, that out of the box Laravel 7 and 8 have the following in the composer.json:

    "autoload": {
        "psr-4": {
            "App\\": "app/",
            "Database\\Factories\\": "database/factories/",
            "Database\\Seeders\\": "database/seeders/"
        }
    },

so on a Vanilla install, the generate command is likely to fail... :(

It is not. Look at the answer above.

SlyDave commented 3 years ago

Can I see your helpers.php file? Also, is that Laravel 8? The factories are actual classes since 7(8?).

Yeah Laravel 8, upgraded from 3 to 4 to 5 to 6 to 7 to 8 mind...

It's as you'd expect, a mess of global functions, here is a snippet:

<?php

use App\Http\Controllers\Shared\MonthReviewController;
use App\Models\Vessel;
use Carbon\Carbon;

/**
 * Gets the currently selected vessel id for the monitoring system
 *
 * @deprecated Don't use selectedVesselId, use selectedVessel
 */
function selectedVesselId() : ?int
{
    $vesselId = Session::get('selectedVesselId');
    return $vesselId ? (int)$vesselId : null;
}

/**
 * @deprecated
 * 
 * Fill an array with $num number of values, and set the keys to be incremented by $key_spacing, starting from $start
 *
 * @param $start
 * @param $num
 * @param $value
 * @param $key_spacing
 * @return array
 */
function array_fill_with_spaced_keys($start, $num, $value, $key_spacing)
{
    $output = [];
    for ($i = 0; $i < $num; $i++) {
        $output[$start + ($i * $key_spacing)] = $value;
    }
    return $output;
}

/**
 * @deprecated
 * 
 * Implodes a multidimensional array
 *
 * @param $array
 * @param $glue
 * @return string
 */
function multi_implode($array, $glue) {
    $ret = '';

    foreach ($array as $item) {
        if (is_array($item)) {
            $ret .= '('. multi_implode($item, $glue) . '),';
        } elseif (is_numeric($item)) {
            $ret .= $item.$glue;
        } else {
            $ret .= "'".$item."'".$glue;
        }
    }

    return substr($ret, 0, strlen($ret)-1);
}

/**
 * @deprecated
 * 
 * Unsets all key-value pairs wih a value of NULL
 *
 * @param array $array
 * @return array
 */
function array_unset_null(array $array)
{
    return array_filter($array, function ($var) {
        return !is_null($var);
    });
}
lepikhinb commented 3 years ago

Yeah, you probably need to wrap the helpers into

if (!function_exists('multi_implode')) {
    function multi_implode() {
        //
    }
}

That's the correct way of writing helpers. Not sure about factories though, it does seem to comply with PSR-4. In older versions of Laravel (which syntax is used in your example) you'd need to specify them in the classmap section:

{
    "autoload": {
        "classmap": [
            "database/seeds",
            "database/factories"
        ],
    }
}
SlyDave commented 3 years ago
$ php artisan typescript:generate
TypeScript definitions generated successfully

Legend.

Thank you so much for punting me in the right direction!

lepikhinb commented 3 years ago

Glad this helped!