Skip to content

Allow to define multiple entrypoints and load assets from each of them. #17

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

Merged
merged 6 commits into from
Dec 20, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions src/Asset/EntrypointLookupCollection.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<?php

/*
* This file is part of the Symfony WebpackEncoreBundle package.
* (c) Fabien Potencier <[email protected]>
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\WebpackEncoreBundle\Asset;

use Symfony\WebpackEncoreBundle\Exception\UndefinedBuildException;
use Psr\Container\ContainerInterface;

/**
* Aggregate the different entry points configured in the container.
*
* Retrieve the EntrypointLookup instance from the given key.
*
* @final
*/
class EntrypointLookupCollection
{
private $buildEntrypoints;

public function __construct(ContainerInterface $buildEntrypoints)
{
$this->buildEntrypoints = $buildEntrypoints;
}

public function getEntrypointLookup(string $buildName): EntrypointLookupInterface
{
if (!$this->buildEntrypoints->has($buildName)) {
throw new UndefinedBuildException(sprintf('Given entry point "%s" is not configured', $buildName));
}

return $this->buildEntrypoints->get($buildName);
}
}
37 changes: 29 additions & 8 deletions src/Asset/TagRenderer.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,39 @@
namespace Symfony\WebpackEncoreBundle\Asset;

use Symfony\Component\Asset\Packages;
use Symfony\Component\DependencyInjection\ServiceLocator;

final class TagRenderer
{
private $entrypointLookup;
private $entrypointLookupCollection;

private $packages;

public function __construct(EntrypointLookupInterface $entrypointLookup, Packages $packages)
{
$this->entrypointLookup = $entrypointLookup;
public function __construct(
$entrypointLookupCollection,
Packages $packages
Copy link
Member

Choose a reason for hiding this comment

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

This reordering of the args breaks BC. What if we did this:

  1. Replace the first arg entirely with EntrypointLookupCollection
  2. But, don't add the type-hint for EntrypointLookupCollection (as that would be a BC break). Instead, we would do an instanceof check in the constructor: if the user is passing an EntrypointLookupInterface, we would trigger a BC warning (example: https://github.com/symfony/symfony/blob/63d34515a51819a6a6b3a94d598e8e37068e85ce/src/Symfony/Component/HttpKernel/DataCollector/ConfigDataCollector.php#L36). In this situation, we would instantiate a new EntrypountLookupCollection() and pass in just the one EntrypointLookupInterface object. If they pass something that is neither of these 2 types, we would throw a TypeError

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I understand the point about BC breaks 😄

Only one question about the exception message. For the TypeError, I think:

The "$entrypointLookupCollection" argument must be an instance of EntrypointLookupCollection.

For the BC warning :

The "$entrypointLookupCollection" argument in method "__construct()" must be an instance of EntrypointLookupCollection.

We do you think ?

) {
if ($entrypointLookupCollection instanceof EntrypointLookupInterface) {
@trigger_error(sprintf('The "$entrypointLookupCollection" argument in method "%s()" must be an instance of EntrypointLookupCollection.', __METHOD__), E_USER_DEPRECATED);

$this->entrypointLookupCollection = new EntrypointLookupCollection(
new ServiceLocator(['_default' => function() use ($entrypointLookupCollection) {
return $entrypointLookupCollection;
}])
);
} elseif ($entrypointLookupCollection instanceof EntrypointLookupCollection) {
$this->entrypointLookupCollection = $entrypointLookupCollection;
} else {
throw new \TypeError('The "$entrypointLookupCollection" argument must be an instance of EntrypointLookupCollection.');
}

$this->packages = $packages;
}

public function renderWebpackScriptTags(string $entryName, string $packageName = null): string
public function renderWebpackScriptTags(string $entryName, string $packageName = null, string $entrypointName = '_default'): string
{
$scriptTags = [];
foreach ($this->entrypointLookup->getJavaScriptFiles($entryName) as $filename) {
foreach ($this->getEntrypointLookup($entrypointName)->getJavaScriptFiles($entryName) as $filename) {
$scriptTags[] = sprintf(
'<script src="%s"></script>',
htmlentities($this->getAssetPath($filename, $packageName))
Expand All @@ -36,10 +52,10 @@ public function renderWebpackScriptTags(string $entryName, string $packageName =
return implode('', $scriptTags);
}

public function renderWebpackLinkTags(string $entryName, string $packageName = null): string
public function renderWebpackLinkTags(string $entryName, string $packageName = null, string $entrypointName = '_default'): string
{
$scriptTags = [];
foreach ($this->entrypointLookup->getCssFiles($entryName) as $filename) {
foreach ($this->getEntrypointLookup($entrypointName)->getCssFiles($entryName) as $filename) {
$scriptTags[] = sprintf(
'<link rel="stylesheet" href="%s">',
htmlentities($this->getAssetPath($filename, $packageName))
Expand All @@ -60,4 +76,9 @@ private function getAssetPath(string $assetPath, string $packageName = null): st
$packageName
);
}

private function getEntrypointLookup(string $buildName): EntrypointLookupInterface
{
return $this->entrypointLookupCollection->getEntrypointLookup($buildName);
}
}
14 changes: 14 additions & 0 deletions src/DependencyInjection/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
use Symfony\Component\Config\Definition\Builder\TreeBuilder;
use Symfony\Component\Config\Definition\ConfigurationInterface;
use Symfony\Component\Config\Definition\Exception\InvalidDefinitionException;

final class Configuration implements ConfigurationInterface
{
Expand All @@ -27,6 +28,19 @@ public function getConfigTreeBuilder()
->isRequired()
->info('The path where Encore is building the assets - i.e. Encore.setOutputPath()')
->end()
->arrayNode('builds')
->useAttributeAsKey('name')
->scalarPrototype()
->validate()
->always(function ($values) {
if (isset($values['_default'])) {
throw new InvalidDefinitionException("Key '_default' can't be used as build name.");
}

return $values;
})
->end()
->end()
->end()
;

Expand Down
22 changes: 21 additions & 1 deletion src/DependencyInjection/WebpackEncoreExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
use Symfony\Component\HttpKernel\DependencyInjection\Extension;
use Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\WebpackEncoreBundle\Asset\EntrypointLookup;

final class WebpackEncoreExtension extends Extension
{
Expand All @@ -24,7 +28,23 @@ public function load(array $configs, ContainerBuilder $container)
$configuration = $this->getConfiguration($configs, $container);
$config = $this->processConfiguration($configuration, $configs);

$factories = [
'_default' => new Reference($this->entrypointFactory($container, '_default', $config['output_path']))
];
foreach ($config['builds'] as $name => $path) {
$factories[$name] = new Reference($this->entrypointFactory($container, $name, $path));
};

$container->getDefinition('webpack_encore.entrypoint_lookup')
->replaceArgument(0, $config['output_path'].'/entrypoints.json');
->replaceArgument(0, $factories['_default']);
$container->getDefinition('webpack_encore.entrypoint_lookup_collection')
->replaceArgument(0, ServiceLocatorTagPass::register($container, $factories));
}

private function entrypointFactory(ContainerBuilder $container, string $name, string $path): string
{
$id = sprintf('webpack_encore.entrypoint_lookup[%s]', $name);
$container->setDefinition($id, new Definition(EntrypointLookup::class, [$path.'/entrypoints.json']));
return $id;
}
}
14 changes: 14 additions & 0 deletions src/Exception/UndefinedBuildException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php

/*
* This file is part of the Symfony WebpackEncoreBundle package.
* (c) Fabien Potencier <[email protected]>
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\WebpackEncoreBundle\Exception;

class UndefinedBuildException extends \InvalidArgumentException
{
}
6 changes: 5 additions & 1 deletion src/Resources/config/services.xml
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,12 @@
<service id="webpack_encore.entrypoint_lookup" class="Symfony\WebpackEncoreBundle\Asset\EntrypointLookup">
<argument /> <!-- entrypoints.json path -->
</service>
<service id="webpack_encore.entrypoint_lookup_collection" class="Symfony\WebpackEncoreBundle\Asset\EntrypointLookupCollection">
<argument /> <!-- build list of entrypoints path -->
</service>

<service id="webpack_encore.tag_renderer" class="Symfony\WebpackEncoreBundle\Asset\TagRenderer">
<argument type="service" id="webpack_encore.entrypoint_lookup" />
<argument type="service" id="webpack_encore.entrypoint_lookup_collection" />
<argument type="service" id="assets.packages" />
</service>

Expand All @@ -23,6 +26,7 @@
<tag name="container.service_locator" />
<argument type="collection">
<argument key="webpack_encore.entrypoint_lookup" type="service" id="webpack_encore.entrypoint_lookup" />
<argument key="webpack_encore.entrypoint_lookup_collection" type="service" id="webpack_encore.entrypoint_lookup_collection" />
<argument key="webpack_encore.tag_renderer" type="service" id="webpack_encore.tag_renderer" />
</argument>
</service>
Expand Down
21 changes: 11 additions & 10 deletions src/Twig/EntryFilesTwigExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,33 +34,34 @@ public function getFunctions()
];
}

public function getWebpackJsFiles(string $entryName): array
public function getWebpackJsFiles(string $entryName, string $entrypointName = '_default'): array
{
return $this->getEntrypointLookup()
return $this->getEntrypointLookup($entrypointName)
->getJavaScriptFiles($entryName);
}

public function getWebpackCssFiles(string $entryName): array
public function getWebpackCssFiles(string $entryName, string $entrypointName = '_default'): array
{
return $this->getEntrypointLookup()
return $this->getEntrypointLookup($entrypointName)
->getCssFiles($entryName);
}

public function renderWebpackScriptTags(string $entryName, string $packageName = null): string
public function renderWebpackScriptTags(string $entryName, string $packageName = null, string $entrypointName = '_default'): string
{
return $this->getTagRenderer()
->renderWebpackScriptTags($entryName, $packageName);
->renderWebpackScriptTags($entryName, $packageName, $entrypointName);
}

public function renderWebpackLinkTags(string $entryName, string $packageName = null): string
public function renderWebpackLinkTags(string $entryName, string $packageName = null, string $entrypointName = '_default'): string
{
return $this->getTagRenderer()
->renderWebpackLinkTags($entryName, $packageName);
->renderWebpackLinkTags($entryName, $packageName, $entrypointName);
}

private function getEntrypointLookup(): EntrypointLookupInterface
private function getEntrypointLookup(string $entrypointName): EntrypointLookupInterface
{
return $this->container->get('webpack_encore.entrypoint_lookup');
return $this->container->get('webpack_encore.entrypoint_lookup_collection')
->getEntrypointLookup($entrypointName);
Copy link
Member

Choose a reason for hiding this comment

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

Once we make the EntrypointCollection also able to handle the "default" build, this should simplify

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I've removed the lookup / collection usage to rely only on collection.

}

private function getTagRenderer(): TagRenderer
Expand Down
20 changes: 20 additions & 0 deletions tests/Asset/EntrypointLookupCollectionTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php

namespace Symfony\WebpackEncoreBundle\Tests\Asset;

use Symfony\Component\DependencyInjection\ServiceLocator;
use Symfony\WebpackEncoreBundle\Asset\EntrypointLookupCollection;
use PHPUnit\Framework\TestCase;

class EntrypointLookupCollectionTest extends TestCase
{
/**
* @expectedException Symfony\WebpackEncoreBundle\Exception\UndefinedBuildException
* @expectedExceptionMessage Given entry point "something" is not configured
*/
public function testExceptionOnMissingEntry()
{
$collection = new EntrypointLookupCollection(new ServiceLocator([]));
$collection->getEntrypointLookup('something');
}
}
38 changes: 38 additions & 0 deletions tests/Asset/EntrypointLookupTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,18 @@ public function testGetJavaScriptFiles()
['file1.js', 'file2.js'],
$this->entrypointLookup->getJavaScriptFiles('my_entry')
);

$this->assertEquals(
[],
$this->entrypointLookup->getJavaScriptFiles('my_entry')
);

$this->entrypointLookup->reset();

$this->assertEquals(
['file1.js', 'file2.js'],
$this->entrypointLookup->getJavaScriptFiles('my_entry')
);
}

public function testGetJavaScriptFilesReturnsUniqueFilesOnly()
Expand Down Expand Up @@ -79,6 +91,32 @@ public function testEmptyReturnOnValidEntryNoJsOrCssFile()
);
}

/**
* @expectedException \InvalidArgumentException
* @expectedExceptionMessageContains There was a problem JSON decoding the
*/
public function testExceptionOnInvalidJson()
{
$filename = tempnam(sys_get_temp_dir(), 'WebpackEncoreBundle');
file_put_contents($filename, "abcd");

$this->entrypointLookup = new EntrypointLookup($filename);
$this->entrypointLookup->getJavaScriptFiles('an_entry');
}

/**
* @expectedException \InvalidArgumentException
* @expectedExceptionMessageContains Could not find an "entrypoints" key in the
*/
public function testExceptionOnMissingEntrypointsKeyInJson()
{
$filename = tempnam(sys_get_temp_dir(), 'WebpackEncoreBundle');
file_put_contents($filename, "{}");

$this->entrypointLookup = new EntrypointLookup($filename);
$this->entrypointLookup->getJavaScriptFiles('an_entry');
}

/**
* @expectedException \InvalidArgumentException
* @expectedExceptionMessage Could not find the entrypoints file
Expand Down
Loading