Improve view helper declaration

[exptom]

Allow view helper to be accessed based on class constant name. Add alias for backwards compatibility.

I was having trouble with the 'asset' alias pulling out the zend-view helper rather than the helper in this module.

[exptom]

It's just the HHVM checks that have failed with error: "HHVM is no longer supported on Ubuntu Precise. Please consider using Trusty with dist: trusty."

[RWOverdijk]

I'm sorry, I don't understand the purpose of this PR. What's wrong with the way it was? It seems to work for many others just fine.

[exptom]

@RWOverdijk This brings the view helper declaration in line with the helpers shipped with ZF3. The main reason is that I have not been able to retrieve the view helper using the name 'asset'.

$viewHelperPluginManager->get('asset') returns the built in zend-view Asset view helper (despite me including this module after the zend-view module.

With my proposed change I can retrieve the helper using the class constant like so: $viewHelperPluginManager->get(AssetManager\View\Helper\Asset::class)

The alias of asset is still present so will not break BC.

[RWOverdijk]

I know that @wshafer is working on the zf3 version. So, ping @wshafer :)

As to the PR, I'll dig up an old project to see if it still works properly. Would you mind adding a test for both cases so we can confirm it stays functional?

[exptom]

@RWOverdijk I've added tests. Would you consider tagging this in a 1.7.2 release? as I could do with this change in my current project.

[RWOverdijk]

@exptom I could. I'll look at it later though, I'm in the middle of something.

[wshafer]

Damn I really have to get my GH notifications working. They keep going to spam and I never see them.

While this is now in the 1.x versions, I'm having a hard time convincing myself to move this to the 2.x version. I'm concerned because this change now deliberately overwrites the documented ZF version of asset.

Is that expected behavior and is that what we want going forward? Or would it be better to use a unique alias, or simply let one overwrite it in config.local.php themselves?

@RWOverdijk @exptom - Any thoughts on this?

[RWOverdijk]

@wshafer I'm not sure I follow. This PR has been merged and released under 1.7.2.

[wshafer]

ZF has a view helper already under the asset name. So if you're following ZF and wanting to use their asset mapper for cache blockers you're out of luck if you install this module.

Before installing Asset Manager you get $this->asset() <- This would be ZF asset view helper

After installing Asset Manager you get $this->asset() <- This would now be the asset managers view helper.

[RWOverdijk]

Hm. You're right. That's silly... I feel like the best way forward here, is by going backward. Reverting this. It's breaking behavior.

If someone really wants this, they can override it in their own application. Agreed? :)

[wshafer]

I agree.

[RWOverdijk]

@exptom I'll be reverting this change.

[exptom]

@RWOverdijk @wshafer Sorry for the delay in reply. I have been unwell and offline for a while.

I'm not sure I follow. This is adding a declaration for the view helper using the class constant (as per ZF3 recommendations) and moves the current factory name of 'asset' to the alias declaration instead. The module already had a conflicting view helper name.

[wshafer]

@exptom - Yes you're correct. However this was a recent problem. The ZF Asset Plugin wasn't available till ZF 3. I'm not sure any of us here knew that plugin was added and that we had a naming collision.

I still don't think it's right to overwrite theirs (even though AM view helper had that namespace first). I have a new AM version almost ready, perhaps we can put it in there under a new namespace... perhaps "assetManager" might work?

[exptom]

@wshafer Ah I see, I hadn't appreciated that their asset view helper was new to ZF3. This all makes sense now :-)

I think renaming in your new version makes sense, is it a full rewrite? Will there be much that I need to change to move to it?

[RWOverdijk]

@exptom Given the amount of code I reviewed already, I can say it's basically a rewrite. @wshafer is not moving away from the concept too far. In fact, I remember him saying it'll be largely BC for ZF2. It will be split up into core, and version specific repositories.

[wshafer]

Biggest change will be if you have any filters attached to extensions. So something like this will need changed:

'asset_manager' => [
     'filters' => [
         'css' => [
             ['filter' => 'Lessphp'],
             ['filter' => 'CssMin'],
         ],
    ],
]

Changes to this (which is what it should be imo)

'asset_manager' => [
     'filters' => [
         'less' => [
             ['filter' => 'Lessphp'],
             ['filter' => 'CssMin'],
         ],
    ],
]

This change also relates to mimetype changes, where we no longer get the mimetype from the source files, we get it from the requested extension of the asset. In most cases this shouldn't break anything, but .less and .scss will now give the correct mimetypes NOT text/css which they incorrectly give now.

There'll be a migration guide and change log available once we push it for beta testing. Which I'm hoping will be soon. 🤞

[exptom]

@wshafer @RWOverdijk thanks for your hard work on the module, I'll look forward to the new version.