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

Split asset compilation #3326

Merged
merged 16 commits into from
Mar 24, 2023
86 changes: 83 additions & 3 deletions recipe/magento2.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,34 @@

// You can also set the themes to run against. By default it'll deploy
// all themes - `add('magento_themes', ['Magento/luma', 'Magento/backend']);`
// If the themes are set as a simple list of strings, then all languages defined in {{static_content_locales}} are
// compiled for the given themes.
// Alternatively The themes cna be defined as an associative array, where the key represents the theme name and
peterjaap marked this conversation as resolved.
Show resolved Hide resolved
// the key contains the languages for the compilation (for this specific theme)
// Example:
// set('magento_themes', ['Magento/luma']); - Will compile this theme with every language from {{static_content_locales}}
// set('magento_themes', [
// 'Magento/luma' => null, - Will compile all languages from {{static_content_locales}} for Magento/luma
// 'Custom/theme' => 'en_US fr_FR' - Will compile only en_US and fr_FR for Custom/theme
// 'Custom/another' => '{{static_content_locales}} it_IT' - Will compile all languages from {{static_content_locales}} + it_IT for Custom/another
// ]); - Will compile this theme with every language
set('magento_themes', [

]);

// Static content deployment options, e.g. '--no-parent'
set('static_deploy_options', '');

// Deploy frontend and adminhtml together as default
set('split_static_deployment', false);

// Use the defualt languages for the backend as default
peterjaap marked this conversation as resolved.
Show resolved Hide resolved
set('static_content_locales_backend', '{{static_content_locales}}');

// backend themes to deploy. Only used if split_static_deployment=true
// This setting supports the same options/structure as {{magento_themes}}
set('magento_themes_backend', ['Magento/backend']);
Copy link
Contributor

Choose a reason for hiding this comment

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

the default value should be ['Magento/backend' => null]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you are right. I'll fix this later

Copy link
Contributor

Choose a reason for hiding this comment

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

I have committed the change for this.


// Configuration

// Also set the number of conccurent jobs to run. The default is 1
Expand Down Expand Up @@ -97,17 +121,73 @@

desc('Deploys assets');
task('magento:deploy:assets', function () {

$themesToCompile = '';
if (count(get('magento_themes')) > 0) {
if (get('split_static_deployment')) {
invoke('magento:deploy:assets:adminhtml');
invoke('magento:deploy:assets:frontend');
} elseif (count(get('magento_themes')) > 0 ) {
foreach (get('magento_themes') as $theme) {
$themesToCompile .= ' -t ' . $theme;
}
run("{{bin/php}} {{release_or_current_path}}/bin/magento setup:static-content:deploy --content-version={{content_version}} {{static_deploy_options}} {{static_content_locales}} $themesToCompile -j {{static_content_jobs}}");
}
});

desc('Deploys assets for backend only');
task('magento:deploy:assets:adminhtml', function () {
magentoDeployAssetsSplit('backend');
});

run("{{bin/php}} {{release_or_current_path}}/bin/magento setup:static-content:deploy --content-version={{content_version}} {{static_content_locales}} $themesToCompile -j {{static_content_jobs}}");
desc('Deploys assets for frontend only');
task('magento:deploy:assets:frontend', function () {
magentoDeployAssetsSplit('frontend');
});

/**
* @param string $area
*
* @phpstan-param 'frontend'|'backend' $area
*
* @throws \Deployer\Exception\RuntimeException
*/
function magentoDeployAssetsSplit(string $area)
{
if (!in_array($area, ['frontend', 'backend'], true)) {
throw new \Deployer\Exception\RuntimeException("\$area must be either 'frontend' or 'backend', '$area' given");
}

$isFrontend = $area === 'frontend';
$suffix = $isFrontend
? ''
: '_backend';

$themesConfig = get("magento_themes$suffix");
$defaultLanguages = get("static_content_locales$suffix");
$useDefaultLanguages = array_is_list($themesConfig);

/** @var list<string> $themes */
$themes = $useDefaultLanguages
? array_values($themesConfig)
: array_keys($themesConfig);

$staticContentArea = $isFrontend
? 'frontend'
: 'adminhtml';

if ($useDefaultLanguages) {
$themes = implode('-t ', $themes);

run("{{bin/php}} {{release_or_current_path}}/bin/magento setup:static-content:deploy --area=$staticContentArea --content-version={{content_version}} {{static_deploy_options}} $defaultLanguages $themes -j {{static_content_jobs}}");
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add --force here by default? Otherwise deploy mode has to be "production" and on a build server you usually don't have that configured

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this makes sense

Copy link
Contributor

Choose a reason for hiding this comment

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

The implode should be also reviewed.
If I have this:

array (
  0 => 'Hyva/default',
  1 => 'Magento/luma',
)

The implode makes this:

Hyva/default-t Magento/luma

However it should be this:

-t Hyva/default -t Magento/luma

Copy link
Contributor

Choose a reason for hiding this comment

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

the implode is updated and the force option is added as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

@akosglue great, could you also fix the merge conflicts?

Copy link
Contributor

Choose a reason for hiding this comment

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

seems to be resolved now

Copy link
Contributor

Choose a reason for hiding this comment

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

@peterjaap any chance to merge this?

return;
}

foreach ($themes as $theme) {
$languages = parse($themesConfig[$theme] ?? $defaultLanguages);

run("{{bin/php}} {{release_or_current_path}}/bin/magento setup:static-content:deploy --area=$staticContentArea --content-version={{content_version}} {{static_deploy_options}} $languages -t $theme -j {{static_content_jobs}}");
}
}

desc('Syncs content version');
task('magento:sync:content_version', function () {
$timestamp = time();
Expand Down