NPM Resource Changed ? Pending

User tests: Successful: Unsuccessful:

avatar Fedik
Fedik
3 Aug 2019

Summary of Changes

As result of discussion in #25309 I have decided to make some changes.
The changes compared to existing WebAsset logic:

  • Now the assets are managed separately per type: script, style etc. (possible to add webcomponents also)
  • WebAssetItem represent a single file (instead of Collection of js, css, as before). This also change JSON schema, and simplify attributes editing (@wilsonge I will do separated pull later)
  • The Collection of js, css now represents by "empty" WebAssetItem with "cross-dependency" (called 'preset')
  • An "attach" method was removed, now it part of rendering process
  • AssetManager now allow to "register" an asset file as AssetItem, eg registerScript($asset)/registerStyle($asset)/registerAsset('foobar', $asset)

Questions that need to discuss:

Asset rendering and backward compatibility.
In current implementation, I have put all assets to _scripts/_stylesheet property of document (before rendering).
This allow to have all scripts/styles onBeforeCompileHead , but this also has a drawback.
It not allow to use AssetManager while this event (see #24858 and #24848).

So I think maybe we can break here a bit?
I mean, do not merge Asset files with _scripts/_stylesheet, but keep them in AssetManager.
Potential b.c. here that _scripts/_stylesheet will not contain files from active Asset at onBeforeCompileHead. (it also allow to drop WebAssetBeforeAttachEvent as useless)
Thoughts?

(I have split them)

Inline scrips/styles
I have skipped it for now, this part require full drop of _script/_style and move to AssetManager.
But because now an assets are managed separately (per type) Inline scrips/styles should be more easy to implement it in future.
In theory it is possible just ignore these properties, and proxy add[Script/Style]Declaration, but I am not sure currently how it will affect 3d extensions.
Thoughts?

(This is for another pull)

Other questions:
Make webcomponents to use AssetManager (no b.c.) Yes/No? (see #27268)
Proxy scriptOptions to AssetManager (minimum b.c.) Yes/No? (can be in another pull)

Testing Instructions

Apply patch run npm install and make sure everything still work as before

Methods comparison

For whom interesting

AssetManager WP
$wa->register[Script,Style,Foobar]() wp_register_[script,style]()
$wa->use[Script,Style,Foobar]()
$wa->registerAndUse[Script,Style,Foobar]()
wp_enqueue_[script,style]()
$wa->disable[Script,Style,Foobar]() wp_dequeue_[script,style]()
n.a. wp_add_inline_[script,style]()

Documentation Changes Required

yeap

@wilsonge @mbabker please review

For references:

doc by @mbabker Joomla Document/WebAsset Analysis
previous pulls #23463 #22435

avatar Fedik Fedik - open - 3 Aug 2019
avatar Fedik Fedik - change - 3 Aug 2019
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 3 Aug 2019
Category Administration com_categories com_modules Templates (admin) JavaScript Repository NPM Change Front End com_finder Layout Libraries
2c8d470 4 Aug 2019 avatar Fedik dog
avatar Fedik Fedik - change - 4 Aug 2019
Labels Added: NPM Resource Changed ?
avatar Fedik Fedik - change - 4 Aug 2019
The description was changed
avatar Fedik Fedik - edited - 4 Aug 2019
avatar Fedik Fedik - change - 4 Aug 2019
The description was changed
avatar Fedik Fedik - edited - 4 Aug 2019
avatar Fedik Fedik - change - 5 Aug 2019
The description was changed
avatar Fedik Fedik - edited - 5 Aug 2019
avatar Fedik Fedik - change - 5 Aug 2019
The description was changed
avatar Fedik Fedik - edited - 5 Aug 2019
avatar Fedik Fedik - change - 5 Aug 2019
The description was changed
avatar Fedik Fedik - edited - 5 Aug 2019
avatar infograf768
infograf768 - comment - 14 Aug 2019

This solves the issue I had with specific xx-XX.css admin lang which is now loaded after bootstrap and template css.

	<link href="/installmulti/joomla40/media/system/css/fields/switcher.min.css?a1bacb193a9e68d6496252f11a1d20e4" rel="stylesheet" />
	<link href="/installmulti/joomla40/administrator/templates/atum/css/bootstrap.min.css?a1bacb193a9e68d6496252f11a1d20e4" rel="stylesheet" />
	<link href="/installmulti/joomla40/administrator/templates/atum/css/template-rtl.min.css?a1bacb193a9e68d6496252f11a1d20e4" rel="stylesheet" />
	<link href="/installmulti/joomla40/administrator/language/fa-IR/fa-IR.css?a1bacb193a9e68d6496252f11a1d20e4" rel="stylesheet" />
	<link href="/installmulti/joomla40/administrator/templates/atum/css/fontawesome.min.css?a1bacb193a9e68d6496252f11a1d20e4" rel="stylesheet" />
etc.
avatar infograf768
infograf768 - comment - 14 Aug 2019

@Fedik
Shall I set this to tested OK as I just tested the css?

avatar Fedik Fedik - change - 15 Aug 2019
The description was changed
avatar Fedik Fedik - edited - 15 Aug 2019
avatar Fedik
Fedik - comment - 15 Aug 2019

@infograf768 yes if other part of the site still works good for you ?
but the whole patch need to be reviewed by @wilsonge and others

avatar Fedik
Fedik - comment - 15 Aug 2019

well, I have to fix a conflict first

avatar wilsonge
wilsonge - comment - 19 Aug 2019

I'm planning on looking at this and I'm aware of it - but I'm really busy with gsoc stuff as it ends this week. 20-30th I'm on holiday so I'm unlikely to be able to properly look at it until after I'm back from holiday. So please don't close this - and sorry it's taking me time to review it properly!

avatar Fedik Fedik - change - 20 Aug 2019
The description was changed
avatar Fedik Fedik - edited - 20 Aug 2019
avatar C-Lodder C-Lodder - test_item - 16 Nov 2019 - Tested successfully
avatar C-Lodder
C-Lodder - comment - 16 Nov 2019

I have tested this item successfully on 5b2e082


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25775.

avatar C-Lodder
C-Lodder - comment - 16 Nov 2019

@Fedik A far better approach for the web assets. Could you fix the conflicts please?

@wilsonge Have you managed to look at this? The sooner a decision is made about this, the better

avatar Fedik Fedik - change - 6 Dec 2019
Labels Added: Conflicting Files
avatar Fedik Fedik - change - 6 Dec 2019
Labels Removed: Conflicting Files
avatar alikon
alikon - comment - 7 Dec 2019

where we are on this @release-joomla postpone to 5.x ?

avatar Fedik
Fedik - comment - 7 Dec 2019

5.x will be to late, it one of "now or never" :)

avatar alikon
alikon - comment - 7 Dec 2019
avatar Fedik
Fedik - comment - 8 Dec 2019

it fixes #27024 and #26077 (comment) also, by 6f6c835

avatar Fedik Fedik - change - 14 Dec 2019
The description was changed
avatar Fedik Fedik - edited - 14 Dec 2019
avatar Fedik Fedik - change - 14 Dec 2019
The description was changed
avatar Fedik Fedik - edited - 14 Dec 2019
avatar Fedik
Fedik - comment - 15 Dec 2019

New feature: an extension now can set template.active as dependency to current template, so their files will be loaded after template style/script files, even if extensions asset was enabled before template render.

avatar SharkyKZ
SharkyKZ - comment - 30 Dec 2019

With PR jquery-noconflict is not loading.

avatar Fedik
Fedik - comment - 30 Dec 2019

It does not required, but you can still load it if you need it

avatar jwaisner jwaisner - test_item - 4 Jan 2020 - Tested successfully
avatar jwaisner
jwaisner - comment - 4 Jan 2020

I have tested this item successfully on 9de1195


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25775.

avatar wilsonge wilsonge - change - 5 Jan 2020
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2020-01-05 17:49:44
Closed_By wilsonge
avatar wilsonge wilsonge - close - 5 Jan 2020
avatar wilsonge wilsonge - merge - 5 Jan 2020
avatar wilsonge
wilsonge - comment - 5 Jan 2020

I have to say with this I'm slightly concerned that we are starting to make things much more complicated in the JSON structure. But at the same time as we're solving concrete problems I'm not sure there's much we can do about it. Can we please get the documentation and the JSON Schema updated please for this new approach

Thankyou for taking the time to investigating michael's review. It's very much appreciated by me :)

avatar Fedik
Fedik - comment - 5 Jan 2020

Schema update are there joomla/schemas#7
But do not merge for now please. I have to update a bit, about webcomponents ( to cover #27268)

I'm slightly concerned that we are starting to make things much more complicated in the JSON structure

yeah, me to, a bit :)
But it not that much complicated as much more content it contain now, because now an asset can contain only one file.
Also, because of this, I have introduced a "preset", a kind of collection that can "hold" a multiple different asset. It more like a "helper".
will see how it will work

the doc page I will look

btw, if extension has 1-2 asset files, it fine just use
$wa->registerAndUseScript('foobar.fancystuff', 'com_foobar/fancy-stuff.js'); instead of JSON.
JSON good for huge amount of assets, as we have in Joomla.

avatar roland-d
roland-d - comment - 3 Feb 2020

@Fedik Did you write up the documentation for the JSON format?

avatar Fedik
Fedik - comment - 3 Feb 2020

Documentation I have updated, it is there https://docs.joomla.org/J4.x:Web_Assets
And JSON schema are there https://github.com/joomla/schemas/blob/master/json-schema/web_assets.json (with properties description)

In documentation there just some examples of JSON, not sure what to write about schema.

avatar roland-d
roland-d - comment - 3 Feb 2020

@Fedik Thank you, I could not find the article on the docs.

avatar Fedik
Fedik - comment - 3 Feb 2020

btw, here is the "public link" to schema https://developer.joomla.org/schemas/json-schema/web_assets.json

avatar roland-d
roland-d - comment - 3 Feb 2020

@Fedik The only thing that is unclear to me is this:

"dependencies": [
        "core",
		"bootstrap"
      ],

How do I know which dependencies are available? The bootstrap is giving a fatal error now. I have no idea where to check what it is looking for. This might be something we can explain in the docs as well.

avatar Fedik
Fedik - comment - 3 Feb 2020

It some how this section https://docs.joomla.org/J4.x:Web_Assets#Register_an_asset
bootstrap actually part of vendors json media/vendor/joomla.asset.json

It should be available on on first access to WebAssetRegistry

But you have to check that file to get a correct name, there a couple of bootstrap variations, eg: bootstrap.css (styles), bootstrap.css.grid (another style), bootstrap.js (script), bootstrap.js.bundle (another script)

I had an idea to extend a debug plugin to print out all available assets, but this for some time later maybe, maybe for 4.2 or so ?

avatar Fedik
Fedik - comment - 3 Feb 2020

and the core asset is part of system json media/system/joomla.asset.json

avatar roland-d
roland-d - comment - 4 Feb 2020

Hey @Fedik Thank you for the feedback so far.

It should be available on on first access to WebAssetRegistry

That doesn't seem to be the case. When I inspect $wa in the index.php of the template, it has some json files loaded but nothing parsed.
image

When I add core as dependency in the templates/cassiopeia/joomla.asset.json file like this:

"name": "template.cassiopeia.ltr",
      "type": "preset",
      "dependencies": [
		"core",
        "template.cassiopeia.ltr#style",
        "template.cassiopeia#script"
      ]

It also bails with an error

Error: Unsatisfied dependency "core" for an asset "template.cassiopeia.ltr" of type "preset": Unsatisfied dependency "core" for an asset "template.cassiopeia.ltr" of type "preset"

It looks like, the json files need to be parsed first before I can load any preset/asset with a dependency. Any ideas about this?

avatar Fedik
Fedik - comment - 4 Feb 2020

It should be available on on first access to WebAssetRegistry
When I inspect $wa in the index.php of the template, it has some json files loaded but nothing parsed.

that correct, they will be parsed on "first access", mean on first $registry->get() call, that will be performed on any $wa->useScript('foobar') call

When I add core as dependency ... It also fails with an error.

That also correct, see what type of asset do you use. If your asset is a script then all dependency also must be script, if your asset are style then all dependency also must be type of style and so on, preset for presets.
There no support of "cross type" dependency (except for presets).

Your example:

"name": "template.cassiopeia.ltr",
"type": "preset", <----- Asset type
"dependencies": [
  "core",   <------ Looking for another "preset" with name "core", not for "script" asset
  "template.cassiopeia.ltr#style",
  "template.cassiopeia#script"
]

As error say, there no "preset" with name "core".
Here is about presets https://docs.joomla.org/J4.x:Web_Assets#Working_with_a_presets with explanation what is special in presets.

Correct would be:

"name": "template.cassiopeia.ltr",
"type": "preset",
"dependencies": [
  "core#script",
  "template.cassiopeia.ltr#style",
  "template.cassiopeia#script"
]

But again, this only for "preset" type, other asset types does not support "cross type" dependency.

avatar roland-d
roland-d - comment - 4 Feb 2020

@Fedik The exteneded explanation is much appreciated. This gives me a lot of insight and it is working now.

avatar Fedik
Fedik - comment - 4 Feb 2020

It was a bit different in first implementation,
In first implementation an asset hold multiple different files css, js, and then after some discussions I have change to handle 1 asset = 1 file

Add a Comment

Login with GitHub to post a comment