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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[rfc] Assets factory, manage JavaScript/StyleSheet collections and their dependency #8913

Closed
wants to merge 44 commits into
base: staging
from

Conversation

Projects
None yet
4 participants
@Fedik
Contributor

Fedik commented Jan 16, 2016

This is the last piece of puzzle: how to throw away JHtml behaviors.
Other parts is #6357 und #3072

How we add js/css in current Joomla API

$doc->addStyleSheet('path/to/style1.css');
$doc->addStyleSheet('path/to/style2.css');
$doc->addScript('path/to/script1.js');
$doc->addScript('path/to/script2.js');
// or
JHtml::_('stylesheet', 'extension/style1.css', false, true);
JHtml::_('script', 'extension/script1.js', false, true);
JHtml::_('script', 'extension/script2.js', false, true);
// or with predefined JHtml behaviors
JHtml::_('behavior.core');
JHtml::_('jquery.framework');
JHtml::_('bootstrap.framework');

Advantage:

  • it works 馃槃
  • it not so complicated (until you reach the point when you need to add a loot css/js)

Drawback

  • I need to remember all dependency. And if script2.js depend from script1.js then they should be added in the correct ordering from the beginning. If script2.js added before script1.js I end up with broken code.
  • if I want to use some library that has css and js I need to add them both in each layout file. And if in some point of time the file name is changed, I need to edit all entry where I added them. Or I should use my own behavior helper.

What I suggest

I suggest to add one more abstraction level 馃懡

  • JHtmlAssetItem represent single asset, and it`s dependency.
  • JHtmlAssetFactory for manage all available assets, end resolve their dependency.

Previous example can become:

JHtml::_('asset.load', 'asset1'); 

Just a single line :neckbeard:

Or with inline script:

JHtml::_('asset.scriptDeclaration', $content, array('asset1')); 

How it works

Each Asset collection defined by joomla.asset.json, that stored aside js/css in the /media folder (see for /media/jui/joomla.asset.json, /media/system/joomla.asset.json for example), or it can be added at the run time (see JHtmlAssetFactory::registerDataFile($path)).

Example, when I want to use bootstrap I call JHtml::_('asset.load', 'bootstrap'); . At this point AssetFactory parse all new joomla.asset.json (if it first call of asset.load) and check whether exist the Asset with this name, and set it to active.

AssetFactory do NOT add any style/script to the document until header render.

Joomla call AssetFactory->attach() before start render the header. At this time AssetFactory check for all active assets and load all their dependency, in case with bootstrap it also will load jquery asset, and then will attach all required css/js files to the document.

Note: For now AssetFactory calculate the dependency only by name and ignore versions.

Advantage:

  • I do not need to remember which asset depend from which, I just call Asset what I need, all else Joomla do for me.
  • it is easy 馃槃
  • as it provide dependency system, it should be more easy to integrate require.js. at time of attach()

Drawback

  • it make a bit more complicated the development entry point
  • I need to write joomla.asset.json, but it is also the Advantage, as I do not need to write my custom behavior helper 馃槃
  • it is to complicated if I really want add just a single file 馃槃

Possible Advantage:

  • If it need, we can allow to provide specific asset class (see JHtmlAssetFactory::prepareAssetInstance()), that will allow to do more tricky stuff (example, attach the files depend from language, etc)
  • with small plugin it allow to request all assets at once (merge them). Like <link href="index.php?com_ajax&getStyleAsset=bootstrap,calendar" > and <script src="index.php?com_ajax&getScriptAsset=bootstrap,calendar" >

API

joomla.asset.json example:

{
    "title" : "Example",
    "name"  : "com_example",
    "author": "Joomla! CMS",
    "assets": [
        {
            "name": "asset1",
            "version": "3.5.0",
            "versionAttach": true,
            "js": [
                "com_example/library1.min.js"
            ]
        },
        {
            "name": "assetname2",
            "version": "3.5.0",
            "js": [
                "com_example/library2.min.js"
            ],
            "css": [
                "com_example/library2.css"
            ],
            "dependency": [
                "core",
                "asset1"
            ],
            "attribute": {
                "com_example/library2.min.js": {
                    "attrname": "attrvalue"
                },
                "com_example/library2.css": {
                    "media": "all"
                }
            }
        },
    ]
}

I hope the Root Elements is clear, all it just for information, and not required.
Children of "assets": [] is actually provided assets .
Each asset can have:

  • name (required) The name of asset. Would be good if it will be compatible with Bower naming.
  • version Optional version of the asset
  • versionAttach Optional. Whether the files will be attached with the version variable (by addScriptVersion/addStyleSheetVersion)
  • js Optional. JavaScript files. Path can be relative to /media folder, relative to Joomla root or external.
  • css Optional. StyleSheet files. Path can be relative to /media folder, relative to Joomla root or external.
  • dependency Optional dependency. Name(s) of assets that required to make current Asset work.
  • attribute Optional attributes that will be attached with related css/js file.

JHtmlAsset helper methods:

// Manage the Assets
JHtml::_('asset.load', 'assetName'); // Load asset by name, or add and load new asset object
JHtml::_('asset.unload',  'assetName'); // Unload asset by name,
JHtml::_('asset.add', $assetInstance); // Add custom asset object to AssetFactory

// Manage the inline js/css
JHtml::_('asset.scriptDeclaration', $content, $dependancy); // Adds an inline script to the page
JHtml::_('asset.styleDeclaration', $content, $dependancy); // Adds an inline style declaration to the page

Final words

It is already worked solution. However I do not replace all Joomla stuff to use Asset factory, as it much work. And before do it, I need to know that this will be accepted.

In combination with #6357 und #3072 (after some refinement there) we can make life more easy (or harder, depend from point of view 馃槃). In theory :neckbeard:

Fedik added some commits Jan 3, 2016

Trigger the onBeforeHeadAttachHtmlAsset event and Attach the assets. 鈥
鈥revent duplicated loop while dependacy resloving.
* @param string $version The asset version
* @param array $owner Asset data file-owner info.
*/
public function __construct($name, $version = null, array $owner = array())

This comment has been minimized.

@andrepereiradasilva

andrepereiradasilva Mar 30, 2016

Contributor

is it possible to generate the assset without the name.

The idea is: i develop an extension and will not need to use the asset name after, we could use:

$asset = new JAssetItem();

And this could generate a random asset identifier.

Even then (if we need) we could also get the random generated asset name with getName later.

@andrepereiradasilva

andrepereiradasilva Mar 30, 2016

Contributor

is it possible to generate the assset without the name.

The idea is: i develop an extension and will not need to use the asset name after, we could use:

$asset = new JAssetItem();

And this could generate a random asset identifier.

Even then (if we need) we could also get the random generated asset name with getName later.

This comment has been minimized.

@Fedik

Fedik Mar 30, 2016

Contributor

The idea was, to be able to interact with assets by name, so you can load/unload the asset at any time until it will be rendered.
Not sure that it is good idea to use random name.

If you very want, then you can use new JAssetItem(uniqid()); 馃槈
It also can make a problem for other extension to identify your asset.

@Fedik

Fedik Mar 30, 2016

Contributor

The idea was, to be able to interact with assets by name, so you can load/unload the asset at any time until it will be rendered.
Not sure that it is good idea to use random name.

If you very want, then you can use new JAssetItem(uniqid()); 馃槈
It also can make a problem for other extension to identify your asset.

This comment has been minimized.

@andrepereiradasilva

andrepereiradasilva Mar 30, 2016

Contributor

the random name was just when we don't need to interact with the asset, just add a js or css file.

@andrepereiradasilva

andrepereiradasilva Mar 30, 2016

Contributor

the random name was just when we don't need to interact with the asset, just add a js or css file.

This comment has been minimized.

@Fedik

Fedik Mar 31, 2016

Contributor

it one of "Drawbacks", when you need just add single file, it should be part of some asset :/

@Fedik

Fedik Mar 31, 2016

Contributor

it one of "Drawbacks", when you need just add single file, it should be part of some asset :/

@andrepereiradasilva

This comment has been minimized.

Show comment
Hide comment
@andrepereiradasilva

andrepereiradasilva Mar 30, 2016

Contributor

@Fedik just saw this PR now.

It seems like a very good idea.
Is the PR this still valid?

If so, some questions:

  • will this allow adding inline js/css with dependencies too?
  • does this allow to add other attributes (e.g. data attributes) to the script or link tag?
  • will this allow to add specific css/js with conditional statements (IE)?
  • with changes in the document rendering, will it be possible to define the asset HTML render position (ex: head tag, before end of body tag)

Drawbacks

  • it make a bit more complicated the development entry point
  • I need to write joomla.asset.json, but it is also the Advantage, as I do not need to write my custom behavior helper 馃槃
  • it is to complicated if I really want add just a single file 馃槃

For what i understand, and please correct if i'm wrong, if want to add a jquery dependent js script (without adding a json file), we'll do this:

$jsAsset = new JAssetItem('myCustomAsset')->setDependency(array('jquery'))->setJs(array('myjsfile.js'));
JHtml::_('asset.load', $jsAsset);

And now we use something like this:

JHtml::_('jquery.framework');       
JHtml::_('script', 'myjsfile.js');

So actually we can add it with the same lines of code (and without json). But yes, is not simple as the current method.

But if this is merged we can in the future modify the addScript (and other script css related methods) to use this to generate the css/js assets, right?

Contributor

andrepereiradasilva commented Mar 30, 2016

@Fedik just saw this PR now.

It seems like a very good idea.
Is the PR this still valid?

If so, some questions:

  • will this allow adding inline js/css with dependencies too?
  • does this allow to add other attributes (e.g. data attributes) to the script or link tag?
  • will this allow to add specific css/js with conditional statements (IE)?
  • with changes in the document rendering, will it be possible to define the asset HTML render position (ex: head tag, before end of body tag)

Drawbacks

  • it make a bit more complicated the development entry point
  • I need to write joomla.asset.json, but it is also the Advantage, as I do not need to write my custom behavior helper 馃槃
  • it is to complicated if I really want add just a single file 馃槃

For what i understand, and please correct if i'm wrong, if want to add a jquery dependent js script (without adding a json file), we'll do this:

$jsAsset = new JAssetItem('myCustomAsset')->setDependency(array('jquery'))->setJs(array('myjsfile.js'));
JHtml::_('asset.load', $jsAsset);

And now we use something like this:

JHtml::_('jquery.framework');       
JHtml::_('script', 'myjsfile.js');

So actually we can add it with the same lines of code (and without json). But yes, is not simple as the current method.

But if this is merged we can in the future modify the addScript (and other script css related methods) to use this to generate the css/js assets, right?

@Fedik

This comment has been minimized.

Show comment
Hide comment
@Fedik

Fedik Mar 30, 2016

Contributor

Is the PR this still valid?

it do not have a conflicts, so I guess it is valid 馃槃

will this allow adding inline js/css with dependencies too?

It only for files, however if we allow "custom class" for each asset https://github.com/joomla/joomla-cms/pull/8913/files#diff-5dbc66d167ed44e3946ee1a79efb5201R574 then the class can have some own logic for add inline css/jss when JAssetFactory::attach() will be called

Also there #6357 is additional idea for reduce to zero the inline js.

  • does this allow to add other attributes (e.g. data attributes) to the script or link tag?
  • will this allow to add specific css/js with conditional statements (IE)?
  • with changes in the document rendering, will it be possible to define the asset HTML render position

The Asset factory it is just a layer over, it does not do more than JDocumentHtml allow to do. See JAssetItem::attachCss and JAssetItem::attachJs

But if this is merged we can in the future modify the addScript (and other script css related methods) to use this to generate the css/js assets, right?

Nope, the reason the same, it just a layer 馃槈
My very naive idea is do not use addScript/addStyleshet directly, in future 馃槃

.. if want to add a jquery dependent js script

yes, you can do as in your example,
or you can make a joomla.asset.json for your extension and place it under media/com_blabla/joomla.asset.json. Then in the code you can add one of your asset by:

JHtml::_('asset.load', 'my-cool-asset-name');

And AssetFactory automatically will load all css/js dependencies.

Also this allow to reuse your asset by other extensions, easily ... in theory 馃槃

Contributor

Fedik commented Mar 30, 2016

Is the PR this still valid?

it do not have a conflicts, so I guess it is valid 馃槃

will this allow adding inline js/css with dependencies too?

It only for files, however if we allow "custom class" for each asset https://github.com/joomla/joomla-cms/pull/8913/files#diff-5dbc66d167ed44e3946ee1a79efb5201R574 then the class can have some own logic for add inline css/jss when JAssetFactory::attach() will be called

Also there #6357 is additional idea for reduce to zero the inline js.

  • does this allow to add other attributes (e.g. data attributes) to the script or link tag?
  • will this allow to add specific css/js with conditional statements (IE)?
  • with changes in the document rendering, will it be possible to define the asset HTML render position

The Asset factory it is just a layer over, it does not do more than JDocumentHtml allow to do. See JAssetItem::attachCss and JAssetItem::attachJs

But if this is merged we can in the future modify the addScript (and other script css related methods) to use this to generate the css/js assets, right?

Nope, the reason the same, it just a layer 馃槈
My very naive idea is do not use addScript/addStyleshet directly, in future 馃槃

.. if want to add a jquery dependent js script

yes, you can do as in your example,
or you can make a joomla.asset.json for your extension and place it under media/com_blabla/joomla.asset.json. Then in the code you can add one of your asset by:

JHtml::_('asset.load', 'my-cool-asset-name');

And AssetFactory automatically will load all css/js dependencies.

Also this allow to reuse your asset by other extensions, easily ... in theory 馃槃

@andrepereiradasilva

This comment has been minimized.

Show comment
Hide comment
@andrepereiradasilva

andrepereiradasilva Mar 30, 2016

Contributor

it do not have a conflicts, so I guess it is valid 馃槃

Since it was travis errors i thought it was "abandoned" :)

I think the concept is very good, and i understand that now we can only use what JDocument allows.
But i think having the possibility to manage all assets (inline or not) through here would be nicer.

If with this we could implement a component with js/css files, inline js/css, dependencies, data-* attributes, async, defer, etc using only Asset factory layer (one to rule then all :) ) i think it would be more easy and logic to everyone, than using a bunch of different methods, ex: JHtml (script, stylesheet, asset), addScript, addScriptVersion, addStyleSheet, etc.

Contributor

andrepereiradasilva commented Mar 30, 2016

it do not have a conflicts, so I guess it is valid 馃槃

Since it was travis errors i thought it was "abandoned" :)

I think the concept is very good, and i understand that now we can only use what JDocument allows.
But i think having the possibility to manage all assets (inline or not) through here would be nicer.

If with this we could implement a component with js/css files, inline js/css, dependencies, data-* attributes, async, defer, etc using only Asset factory layer (one to rule then all :) ) i think it would be more easy and logic to everyone, than using a bunch of different methods, ex: JHtml (script, stylesheet, asset), addScript, addScriptVersion, addStyleSheet, etc.

@Fedik

This comment has been minimized.

Show comment
Hide comment
@Fedik

Fedik Mar 31, 2016

Contributor

But i think having the possibility to manage all assets (inline or not) through here would be nicer.

With assets I mean only files, here.
Example you use colorbox: the library contain colorbox.js and colorbox.css - that the "colorbox" asset.
And the inline code for init the library it is not "asset" .. cause it can be very different, depend from case.

However you can make one more asset for your extension with "behavior-colorbox.js" (see other pull request) with "colorbox" as dependency, and putt there all code that will handle data-*, and do other stuf that need for your extension.

Well I am bad in good explanation 馃槃

Contributor

Fedik commented Mar 31, 2016

But i think having the possibility to manage all assets (inline or not) through here would be nicer.

With assets I mean only files, here.
Example you use colorbox: the library contain colorbox.js and colorbox.css - that the "colorbox" asset.
And the inline code for init the library it is not "asset" .. cause it can be very different, depend from case.

However you can make one more asset for your extension with "behavior-colorbox.js" (see other pull request) with "colorbox" as dependency, and putt there all code that will handle data-*, and do other stuf that need for your extension.

Well I am bad in good explanation 馃槃

@andrepereiradasilva

This comment has been minimized.

Show comment
Hide comment
@andrepereiradasilva

andrepereiradasilva Mar 31, 2016

Contributor

ok, this is only for external js/css files. I understand why, but from a developer learning curve perspective don't you agree with me that can be rather confusing having so many methods for js/css inline external, versioning, etc?

Contributor

andrepereiradasilva commented Mar 31, 2016

ok, this is only for external js/css files. I understand why, but from a developer learning curve perspective don't you agree with me that can be rather confusing having so many methods for js/css inline external, versioning, etc?

@Fedik

This comment has been minimized.

Show comment
Hide comment
@Fedik

Fedik Apr 1, 2016

Contributor

well, yes .. learning is hard here, can be ...
Maybe I try add some proxy for addXXXDeclaration, in to JHtmlAsset, so all will be in the one place.

So after all it can looks like:
Before
for 'some-library':

JHtml::_('behavior.core');
JHtml::_('jquery.framework');
JHtml::_('stylesheet', 'extension/some-library.css', false, true);
JHtml::_('script', 'extension/some-library.js', false, true);
$doc->addScriptDeclaration('some init code');

It not hard if I have it in single layout, but a bit annoying if need to edit it in couple different layouts.
I know about JHtml Behaviors, but it hard to manage the dependencies.

After
joomla.asset.json:

{
    "title" : "Some Extension",
    "name"  : "com_someextension",
    "author": "my name",
    "assets": [
        {
            "name": "some-library",
            "version": "1.0.0",
            "versionAttach": true, // attach the version query to each file
            "js": [
                "com_someextension/some-library.min.js"
            ],
            "css": [
                "com_someextension/some-library.css"
            ],
            "dependency": [
                "core",
                "jquery"
            ]
        },
    ]
}

and in layout just:

JHtml::_('asset.scriptDeclaration', 'alert("blabla")', $deps = array('some-library'));

hm

Contributor

Fedik commented Apr 1, 2016

well, yes .. learning is hard here, can be ...
Maybe I try add some proxy for addXXXDeclaration, in to JHtmlAsset, so all will be in the one place.

So after all it can looks like:
Before
for 'some-library':

JHtml::_('behavior.core');
JHtml::_('jquery.framework');
JHtml::_('stylesheet', 'extension/some-library.css', false, true);
JHtml::_('script', 'extension/some-library.js', false, true);
$doc->addScriptDeclaration('some init code');

It not hard if I have it in single layout, but a bit annoying if need to edit it in couple different layouts.
I know about JHtml Behaviors, but it hard to manage the dependencies.

After
joomla.asset.json:

{
    "title" : "Some Extension",
    "name"  : "com_someextension",
    "author": "my name",
    "assets": [
        {
            "name": "some-library",
            "version": "1.0.0",
            "versionAttach": true, // attach the version query to each file
            "js": [
                "com_someextension/some-library.min.js"
            ],
            "css": [
                "com_someextension/some-library.css"
            ],
            "dependency": [
                "core",
                "jquery"
            ]
        },
    ]
}

and in layout just:

JHtml::_('asset.scriptDeclaration', 'alert("blabla")', $deps = array('some-library'));

hm

@andrepereiradasilva

This comment has been minimized.

Show comment
Hide comment
@andrepereiradasilva

andrepereiradasilva Apr 1, 2016

Contributor

yes, it seems good. or for just jquery dependency something like this.

$inlineJs = '$( document ).ready(function() { alert("blabla"); });';
JHtml::_('asset.scriptDeclaration', $inlineJs, array('jquery'));

or for vanilla js without dependencies something like this.

JHtml::_('asset.scriptDeclaration', 'alert("blabla");');
Contributor

andrepereiradasilva commented Apr 1, 2016

yes, it seems good. or for just jquery dependency something like this.

$inlineJs = '$( document ).ready(function() { alert("blabla"); });';
JHtml::_('asset.scriptDeclaration', $inlineJs, array('jquery'));

or for vanilla js without dependencies something like this.

JHtml::_('asset.scriptDeclaration', 'alert("blabla");');
@Fedik

This comment has been minimized.

Show comment
Hide comment
@Fedik

Fedik Apr 10, 2016

Contributor

I have added:

JHtml::_('asset.scriptDeclaration', $content, $dependancy); // Adds an inline script to the page
JHtml::_('asset.styleDeclaration', $content, $dependancy); // Adds an inline style declaration to the page

And now the AssetFactory pick up joomla.asset.json from the template to (see example for Isis and Protostar, in this pull request)

Contributor

Fedik commented Apr 10, 2016

I have added:

JHtml::_('asset.scriptDeclaration', $content, $dependancy); // Adds an inline script to the page
JHtml::_('asset.styleDeclaration', $content, $dependancy); // Adds an inline style declaration to the page

And now the AssetFactory pick up joomla.asset.json from the template to (see example for Isis and Protostar, in this pull request)

Sync. Merge branch 'master' into assets-factory
Conflicts:
	administrator/templates/isis/index.php
	libraries/cms/html/behavior.php
	libraries/joomla/form/fields/file.php
Sync. Merge branch 'master' into assets-factory-sync
 Conflicts:
	administrator/templates/isis/component.php
	administrator/templates/isis/index.php
	administrator/templates/isis/login.php
	libraries/cms/html/behavior.php
	templates/protostar/component.php
	templates/protostar/index.php
@mbabker

This comment has been minimized.

Show comment
Hide comment
@mbabker

mbabker May 21, 2017

Member

Any word on this?

Member

mbabker commented May 21, 2017

Any word on this?

@Fedik

This comment has been minimized.

Show comment
Hide comment
@Fedik

Fedik May 22, 2017

Contributor

Sadly it cannot be synced easily (more easy to write new). I kept it open to get some feedback, but seems not much interest in it.
I planed to make "basic" version of this patch for J 4.x, maybe some day.

I am closing it,
if someone interesting, can keep discussion in the related topic #12402

Contributor

Fedik commented May 22, 2017

Sadly it cannot be synced easily (more easy to write new). I kept it open to get some feedback, but seems not much interest in it.
I planed to make "basic" version of this patch for J 4.x, maybe some day.

I am closing it,
if someone interesting, can keep discussion in the related topic #12402

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment