hearsayit / HearsayRequireJSBundle

RequireJS integration for Symfony2.
130 stars 55 forks source link

Almond integration #52

Closed sfblaauw closed 10 years ago

sfblaauw commented 10 years ago

Hi, like @jrburke says:

Some developers like to use the AMD API to code modular JavaScript, but after doing an optimized build, they do not want to include a full AMD loader like RequireJS, since they do not need all that functionality...

This is the reason why I believe that it will be nice to have the possibility to use Almond for production environment.

I sent a PR with an approach and you can download and test it in HearsayRequireJSWithAlmond.

There is no test but If you think that can be integrated in HearsayRequireJSBundle I can do it.

Sorry for my english. Thanks.

sfblaauw commented 10 years ago

Maybe this could be a better approach:


# Hearsay\RequireJSBundle\Assetic\Filter\RJsFilter:makeBuildProfile

$buildProfileEvent = new BuildProfileEvent($content);

/** @var Symfony\Component\EventDispatcher\ContainerAwareEventDispatcher */
$eventDispatcher = $this->container->get('event_dispatcher');
$eventDispatcher->dispatch(RequireJSEvents::BUILD_PROFILE_CONTENT_COMPLETED, $buildProfileEvent);

$content = $buildProfileEvent->getContent();

file_put_contents($buildProfile, '(' . json_encode($content) . ')');

return $buildProfile;


namespace Hearsay\RequireJSBundle;

/**
 * Contains all events thrown in the RequireJSBundle
 */
final class RequireJSEvents
{
    /**
    * The BUILD_PROFILE_CONTENT_COMPLETED event occurs after all content data is stored before the build profile process.
    */
    const BUILD_PROFILE_CONTENT_COMPLETED = 'require_js.build_profile.content.completed';
}


namespace Hearsay\RequireJSBundle\Event;

use Symfony\Component\EventDispatcher\Event;

class BuildProfileEvent extends Event
{
    private $content;

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

    public function setContent($content)
    {
        $this->content = $content;
    }

    public function getContent()
    {
        return $this->content;
    }
}


namespace Acme\DemoBundle\EventListener;

use Symfony\Component\EventDispatcher\EventSubscriberInterface;

use Hearsay\RequireJSBundle\RequireJSEvents;
use Hearsay\RequireJSBundle\Event\BuildProfileEvent;

class RequireJsSubscriber implements EventSubscriberInterface
{
   public static function getSubscribedEvents()
    {
        return [
            RequireJSEvents::BUILD_PROFILE_CONTENT_COMPLETED => 'onBuildProfileContentCompleted'
        ];
    }

    public function onBuildProfileContentCompleted(BuildProfileEvent $event)
    {
        $content = $event->getContent();

        $content->include       = array($content->name);
        $content->insertRequire = array($content->name);
        // Almond options
        $content->name          = '../../app/Resources/public/build/almond';
        $content->wrap          = true;

        $event->setContent($content);
    }
}

# requirejs_prod.yml

hearsay_require_js:
    initialize_template: ':requirejs:initialize.html.twig'
    optimizer:
        hide_unoptimized_assets: true
        path: "%kernel.root_dir%/Resources/public/build/r.js"
        options:
            findNestedDependencies: true

# :requirejs:initialize.html.twig

<script type="text/javascript" src="{{ main }}"></script>
ihortymoshenko commented 10 years ago

@sfblaauw, looks like an interesting feature. I like the first solution. Perhaps, it makes sense to add the event in order to have the extension point. I'm not sure.

Did you tested it?

sfblaauw commented 10 years ago

Hi @IgorTimoshenko. No, I didn't test it but I been working with the fork and it works. If you think that this feature can be merged I will like to make some refactoring and the unit test. In my opinion the Event option was made to avoid the almond integration in your bundle and to give the possibility to do a hook in the optimization file, the Almond implementation was only an example, it is therefore I think the Event will be nice to implement. With respect to Almond implantation I don't think that has to use the Event, it has to be in the core. What you think?

ihortymoshenko commented 10 years ago

@sfblaauw, I agree. It should be in the core. I have a few notices about this PR:

Write tests, if necessary. After this let's discuss it again.

I think that's all for now.

P.S. Thanks for the contribution :)

sfblaauw commented 10 years ago

Ok, I agree, but I think that the almond_path has to be under the optimizer node because is related with de optimization and it works with r.js. If there is no r.js you can't use almond. Also there are some options for almond that has to be configured to work properly and must take into account, like insertRequire or wrap. This options are also under the optimization node. Please let me know your opinion and I will start.

sfblaauw commented 10 years ago

Ok, done first approach: https://github.com/sfblaauw/HearsayRequireJSBundle/tree/almond To make it work (works only with optimization):

# config.yml

hearsay_require_js:
    almond_path: /absolute/path/to/almond.js
    optimizer:
        path: /absolute/path/to/r.js
ihortymoshenko commented 10 years ago

@sfblaauw, cool! I've written my thoughts under your last commit. Please add a few lines in docs and send your changes when you will be ready. Thanks!

guilhermeblanco commented 10 years ago

@sfblaauw @IgorTimoshenko You can close this one, since https://github.com/hearsayit/HearsayRequireJSBundle/pull/64 is a replacement of it with requested changes committed.

sfblaauw commented 10 years ago

Thanks @guilhermeblanco and sorry @IgorTimoshenko