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

sylius:theme:assets:install not generating public/_themes directory #91

Open
mbabker opened this issue Mar 3, 2021 · 27 comments
Open

Comments

@mbabker
Copy link
Contributor

mbabker commented Mar 3, 2021

With Sylius 1.9.0 and sylius/theme-bundle 2.1.1, when I run the bin/console sylius:theme:assets:install command (without symlinks due to #79), I am not getting a version of the assets in the public/_themes directory. public/bundles does generate correctly.

For our client, we overloaded some admin field types to use CKEditor through FOSCKEditorBundle and are installing its assets via bin/console ckeditor:install --clear=drop. After running the CKEditor install command then the asset install command and going to any admin page where we have CKEditor in use, the asset URL (as determined with the PathResolver class) is /_themes/<theme_composer_name>/bundles/fosckeditor/ckeditor.js. With Sylius 1.8 and the 1.x version of this bundle, the asset URL is /bundles/_themes/<theme_composer_name>/fosckeditor/ckeditor.js and this is working OK (note, public/bundles/_themes doesn't exist on the new version either, just to be sure there wasn't anything strange here).

@mbabker
Copy link
Contributor Author

mbabker commented Mar 18, 2021

I'm in over my head at this point but basically, it is failing because of the path resolver.

With 1.5.1, this call to the resolver is correctly building the public/bundles/_themes path. With 2.1.1, this call is not. It looks like there is a discrepancy between how the resolver is used in the asset installer versus its use in the PathPackage class, more specifically at the point it's used in the installer it looks to be processing a relative folder name whereas with the PathPackage (and the test cases in the resolver's spec) it expects a full asset path.

The quickest band-aid fix for anyone using a single theme application would probably be to get rid of the assets.path_package service override from this bundle and restore the original service definition from the FrameworkBundle.

@LucaGallinari
Copy link

Hi guys, i saw that the version 2.2.0 has been released but we are still having this issue

@pamil
Copy link
Contributor

pamil commented Mar 19, 2021

v2.2.0 includes support for PHP 8 and the new public resources directory in bundles, I haven't got enough time to debug this one - I'll be happy to review any PR that helps to solve it though.

@mbabker
Copy link
Contributor Author

mbabker commented Mar 19, 2021

Unfortunately, I reached the end of what I could do yesterday. What drives me bonkers is that AssetTest::it_dumps_assets() actually produces the right result, yet in my production application I can't make the thing work to save my life. All I really worked out for sure is that there's something wonky with how the path resolver is used in the installer versus from Symfony's Asset API. Even if I change my commands to use absolute paths instead of relative, it changes nothing.

Hopefully, someone can kind of work out where to go with the extra info I stumbled across.

@jbcr
Copy link

jbcr commented Mar 22, 2021

Hi,

In my case, all assets stored in the public folder public/bundles cannot be linked with path() twig method.

All URL are prefixed by _themes/my/theme and all of these URLs return a 404 error.

When I run sylius:theme:assets:install, only Installing assets as hard copies. is displayed but nothing changes in the public directory.

EDIT: In the administration, if I select a channel with the default theme, no prefix has been added on the assets path. Why?

Symfony: 5.2.5
Sylius: 1.9.1
ThemeBundle: 2.2.0
LegacyMode: true

@jbcr
Copy link

jbcr commented Mar 23, 2021

In my case, I don't have assets on my themes.

To solve this issue, I have disabled assets on themes

sylius_theme:
    assets: 
        enabled: false

@maximehuran
Copy link

This issue is really important I think.
We cannot install our plugins assets on Sylius projects which are using themes.
We have 404 on resources, so no JS or CSS are loaded.

@maxperei
Copy link

maxperei commented Apr 1, 2021

hi guys,

what i thought to understand since this bundle's upgrade :

  • ./themes/ directory at project root must have a public/ folder inside to replicate the structure _themes/[author]/[theme-name]/bundles/**/... in ./public/ directory (cf. ./src/Asset/Installer/AssetsProvider.php:44)

but this induces several disappointments during bin/console syl:the:ass:ins

  1. because [--relative|--symlink] option loses assets (cf. Command sylius:theme:assets:install --symlink --relative sometimes remove origin files #79)
  2. whereas all your custom theme content is disposed or duplicated at the wrong place (inside ./public/_themes/...)
  3. sometimes in particular cases it breaks the way bundle's assets are loaded eg. image

so i think this is more a regression than a discovery, and btw in my opinion i'd rather delete the pathResolver->resolve() call at ./src/Asset/Package/PathPackage.php:57 but maybe its a bit agressive and not justified.. 😅

@jacquesbh
Copy link
Member

@pamil We really have an issue here 🙃.

It's driving me crazy as well. Something is wrong and has to be fixed.

Why is the theme used in admin?

@jacquesbh
Copy link
Member

jacquesbh commented May 2, 2021

Here is a quick fix for those who can't wait:

<?php

declare(strict_types=1);

namespace App\Sylius\Core\Theme;

use Exception;
use Sylius\Bundle\ThemeBundle\Context\ThemeContextInterface;
use Sylius\Bundle\ThemeBundle\Model\ThemeInterface;
use Sylius\Bundle\ThemeBundle\Repository\ThemeRepositoryInterface;
use Sylius\Component\Channel\Context\ChannelContextInterface;
use Sylius\Component\Channel\Context\ChannelNotFoundException;
use Sylius\Component\Core\Model\ChannelInterface;
use Symfony\Component\HttpFoundation\RequestStack;

final class ChannelBasedThemeContext implements ThemeContextInterface
{
    private RequestStack $requestStack;
    private ChannelContextInterface $channelContext;
    private ThemeRepositoryInterface $themeRepository;

    public function __construct(
        RequestStack $requestStack,
        ChannelContextInterface $channelContext,
        ThemeRepositoryInterface $themeRepository
    ) {
        $this->requestStack = $requestStack;
        $this->channelContext = $channelContext;
        $this->themeRepository = $themeRepository;
    }

    public function getTheme(): ?ThemeInterface
    {
        if ($this->isAdmin()) {
            return null;
        }
        try {
            /** @var ChannelInterface $channel */
            $channel = $this->channelContext->getChannel();
            $themeName = $channel->getThemeName();

            if (null === $themeName) {
                return null;
            }

            return $this->themeRepository->findOneByName($themeName);
        } catch (ChannelNotFoundException $exception) {
            return null;
        } catch (Exception $exception) {
            return null;
        }
    }

    private function isAdmin(): bool
    {
        if (null === $masterRequest = $this->requestStack->getMasterRequest()) {
            return false;
        }
        $syliusParameters = $masterRequest->get('_sylius', []);
        return array_key_exists('section', $syliusParameters) && 'admin' === $syliusParameters['section'];
    }
}
services:
    # the class App\Sylius\Core\Theme\ChannelBasedThemeContext is autowired.
    sylius.theme.context.channel_based: '@App\Sylius\Core\Theme\ChannelBasedThemeContext'

@maximehuran
Copy link

@jacquesbh @pamil see #102

@LucaGallinari
Copy link

If it can help someone, we temporarily solved this by using this work around:

composer.json

{
...

    "scripts": {
        "post-install-cmd": [
            ...
            "@symlink-themes"
        ],
        "post-update-cmd": [
            ...
            "@symlink-themes"
        ],
        "symlink-themes": "mkdir -p public/_themes/[THEME_VENDOR]/[THEME_NAME] && ln -s ../../../bundles public/_themes/[THEME_VENDOR]/[THEME_NAME]/bundles",
    }
...
}

@0mars
Copy link

0mars commented May 25, 2021

On a side-note if you're trying to debug this with docker-compose packed with Sylius 1.9 then add this to the php container to stall a bit and not shutdown while failing

# Dockerfile
APK add vim bash
# docker-compose.yml
services:
  php:
    command: bash -c "sleep 5000"

@dsbe-ak
Copy link

dsbe-ak commented Aug 6, 2021

Afaik this is still an issue for sylius 1.10.
Any updates on when this will be fixed?
Thanks!

@citadev
Copy link

citadev commented Oct 11, 2021

Here is another workaround :

For background I use sylius 1.9, with a custom theme (extending the BootstrapTheme), in admin vendor assets are loaded from /_themes/<my>/<theme>/bundles/... which doesn't exist.

I checked sylius:theme:assets:install help, I found that the command accept an optionnal target argument allowing us to choose the target directory where the assets will be written.

First, create public/_themes/<my>/<theme> (or next command will fail).
Then, execute sylius:theme:assets:install public/_themes/<my>/<theme>
You'll get a public/_themes/<my>/<theme>/bundles directory tree with all the vendor assets.

After that, vendor assets in admin are loading just fine.
Just a workaround, but no coding and easy to integrate in a CI/CD pipeline.

Hope that it can help someone.

@stefandoorn
Copy link
Contributor

I'm trying to upgrade to Sylius v1.9, with a layered theme in this project (theme2 extends theme1 extends Sylius default theme), and running into the same issues. All kind of frontend dependencies are missing from the public/_themes folder, which used to work fine under Sylius v1.8.

@jacquesbh
Copy link
Member

ping @Zales0123 @Tomanhez <3

@plewandowski
Copy link

Well, I wonder if theme for admin should not be selected independently from store theme.... right now it works really weird to me.... but it is more feature request :-)

@Neirda24
Copy link

Neirda24 commented Nov 15, 2021

Because it is needed for each themes to run this fix we made a small bash script that should handle it correctly :

#!/usr/bin/env bash

# Related issue : https://github.com/Sylius/SyliusThemeBundle/issues/91
#./bin/fix-theme-assets.sh

# Requirements:
#   - jq ~= 1.6

set -o errexit;
set -o nounset;
set -o pipefail;

goToRoot() {
    local currentDir;
    currentDir=$( cd "$( dirname $0 )" && pwd )

    cd "${currentDir}/../"
}

fix_theme() {
    local themeName="${1:?}"

    mkdir -p "public/_themes/${themeName}";
    unlink "public/_themes/${themeName}/bundles" 2> /dev/null || true;
    ln -s "../../../bundles" "public/_themes/${themeName}/bundles";
}

fix_themes() {
    while IFS= read -r -d '' themeConfig
    do
        local themeName;
        themeName=$(jq 'if .extra? | has("sylius-theme") then .name else null end' -r "${themeConfig}")

        if [ "null" != "${themeName}" ]; then
            fix_theme "${themeName}"
        fi
    done < <(find "./themes/"* -maxdepth 1 -name 'composer.json' -print0)
}

main() {
    fix_themes
}

goToRoot
main

hope it helps someone.

@roromix
Copy link

roromix commented Dec 3, 2021

To solve this problem, you just have to create an empty public directory in each of your themes.

Maybe we should patch sylius/theme-bundle to create the /_themes/author/ theme-name/bundles directory, even though we don't have a public directory.

@stefandoorn
Copy link
Contributor

@roromix is correct, but the files do not end up in bundles/_themes anymore, just straight into _themes folder.

I have the legacy mode enabled (from the upgrade guide) btw.

@stefandoorn
Copy link
Contributor

Commands I used to run on a deployment:

        php bin/console --env=prod --no-debug --ansi assets:install public
        php bin/console --env=prod --no-debug --ansi sylius:install:assets
        php bin/console --env=prod --no-debug --ansi sylius:theme:assets:install public
        yarn run build

I would get a folder structure like:

ls -lah public/bundles/
total 0
drwxr-xr-x 13 web web 286 Dec 24 10:48 .
drwxr-xr-x 11 web web 289 Dec 24 10:50 ..
drwxr-xr-x  3 web web  36 Dec 24 10:48 _themes

ls -lah public/bundles/_themes/
total 0
drwxr-xr-x  3 web web  36 Dec 24 10:48 .
drwxr-xr-x 13 web web 286 Dec 24 10:48 ..
drwxr-xr-x  4 web web  63 Dec 24 10:48 project

ls -lah public/bundles/_themes/project/
total 0
drwxr-xr-x  4 web web  63 Dec 24 10:48 .
drwxr-xr-x  3 web web  36 Dec 24 10:48 ..
drwxr-xr-x 13 web web 308 Dec 24 10:49 base-theme
drwxr-xr-x 11 web web 253 Dec 24 10:48 core-theme

I could reference all my assets like this:

<meta property=“og:image” content=“{{ asset(‘bundles/theme/svg/logo.svg’) }}” />

Which would rewrite into this:

<meta property="og:image" content="/bundles/_themes/project/base-theme/theme/svg/logo.svg" />

Now I need to access them like:

<meta property=“og:image” content=“{{ asset(‘bundles/_themes/project/base-theme/theme/svg/logo.svg’) }}” />

Also I need to add another step in my build process BEFORE the yarn run build:

cp -Rp public/_themes/ public/bundles

Simply to just get a folder like it used to be.

I'm using the legacy_mode as I was hoping to do such upgrades unrelated to Sylius upgrades.

Main issue is here probably that's not documented in the UPGRADE guide for version 2 that assets will be installed somewhere else and that referencing them in templates would also break.

@lchrusciel
Copy link
Member

Hey folks,

I've promised on a slack some time ago, that we will take care of this issue. However, as not a creator of this library it is hard to do deep dive into it. To help you, I need working, ready to use reproducer for your issue. It may be new test case in this project, it may be vanilla Sylius-Standard just with sample project. In addition, please point out the correct way to assert of your expected outcome. I'm aware, that you had to do the same, don't get me wrong. But I also have to help on other ends of Sylius and I won't commit to build reproducer at this moment. Nonetheless, I want to unblock(or just fix it on upstream) you.

In test I trust :)

@mbabker
Copy link
Contributor Author

mbabker commented Jan 12, 2022

I haven't done a deep dive on this in a long while, but in theory, this should be enough to make a reproducer:

  • Fresh clone of Sylius-Standard
  • Make a theme, presumably the composer.json should be enough in the filesystem
  • Run the sylius:theme:assets:install command

When you run Symfony's assets:install command, you should get a public/bundles folder with the public assets from ApiPlatformBundle and BabDevPagerfantaBundle. The sylius:theme:assets:install command should generate a similar filesystem structure for each theme, but I've had this part of the theme bundle disabled for so long I actually don't remember what the expected result should look like anymore 😄 .

@Loocos
Copy link

Loocos commented Dec 29, 2022

By making a public folder inside all my themes with a .gitkeep inside only, the command is now able to write all files and folders in the public folder.
The easiest fix I've found.

@LeoMoshko
Copy link

Any new idea?

@hubertinio
Copy link

The problem still exists, here is my solution because any other don't work.

$services->set('assets.path_package', \App\Asset\PathPackage::class)
      ->args([
          '/',
          service('assets.empty_version_strategy'),
          service(\Sylius\Bundle\ThemeBundle\Context\ThemeContextInterface::class),
          service(\Sylius\Bundle\ThemeBundle\Asset\PathResolverInterface::class),
          service('filesystem'),
          param('kernel.project_dir'),
          service('assets.context'),
      ])
  ;

If packge exists in default folder, dont play with other solutions.

<?php

namespace App\Asset;

use Sylius\Bundle\ThemeBundle\Asset\PathResolverInterface;
use Sylius\Bundle\ThemeBundle\Context\ThemeContextInterface;
use Symfony\Component\Asset\Context\ContextInterface;
use Symfony\Component\Asset\VersionStrategy\VersionStrategyInterface;
use Symfony\Component\Filesystem\Filesystem;

class PathPackage extends \Sylius\Bundle\ThemeBundle\Asset\Package\PathPackage
{
    public function __construct(
        string $basePath,
        VersionStrategyInterface $versionStrategy,
        ThemeContextInterface $themeContext,
        PathResolverInterface $pathResolver,
        private Filesystem $filesystem,
        private string $projectDir,
        private ?ContextInterface $context = null
    ) {
        parent::__construct($basePath, $versionStrategy, $themeContext, $pathResolver, $context);
    }

    public function getUrl($path): string
    {
        $basePath = ltrim($path, DIRECTORY_SEPARATOR);
        $absolutePath = $this->projectDir . DIRECTORY_SEPARATOR
            . 'public' . DIRECTORY_SEPARATOR
            . $basePath;

        if ($this->filesystem->exists($absolutePath)) {
            return $this->context->getBasePath() . '/' . $basePath;
        }

        return parent::getUrl($path);
    }
}

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 a pull request may close this issue.