User tests: Successful: Unsuccessful:
This time has come. After all this years we should stop using fancy compilers and code native JS/CSS as in good old days.
Just joking.
I updated the build script to be able to run it per extension, without rebuilding whole thing each time.
And fixed few bugs on the way.
Main changes:
media_source//build/media_source/ to /media_source. With this all relative includes will be the same now for both media/ and media_source/.media_source/ in to /media_source/README.md.Problem with existing build
media_source.New builder
clear remove existing files in foldercopy copy files to /mediacss compile css/scss filesjs compile js files and modulesRead more in /media_source/README.md for more detail.
Few examples
# build all
npm run build -- -a
# build only com_content assets
npm run build -- -n com_content
# build only admin template
npm run build -- -n templates/administrator/atum
# build only styles of the admin template
npm run build -- -n templates/administrator/atum -t css
# watch on asset
npm run watch -- -n com_content
npm run watch -- -n templates/administrator/atum
# watch on few assets
npm run watch -- -n com_content,com_categories
Run build.
All should work as before.
Please select:
| Status | New | ⇒ | Pending |
| Category | ⇒ | Repository JavaScript |
| Title |
|
||||||
| Labels |
Added:
Feature
PR-6.1-dev
|
||
Good work, what about renaming media_source to resources? Can the watch script also be executed for extensions only?
yes really nice work thanks fedir, I think media_source is ok, it's more clear that media_source will be media we know already, then resources to media.
what about renaming media_source to resources?
I think existing is good. But can rename to anything if we get consensus on new name 😄
Can the watch script also be executed for extensions only?
Yes, it watches only on specified extension (one or few):
npm run watch -- -n com_content
npm run watch -- -n templates/administrator/atum
# on few
npm run watch -- -n com_content,com_categoriesThanks Fedora 💚
Thanks Fedir 💚
| Category | Repository JavaScript | ⇒ | Administration Repository JavaScript |
| Category | Repository JavaScript Administration | ⇒ | Repository JavaScript |
This pull request has conflicts, please resolve those before we can evaluate the pull request.
| Labels |
Added:
Conflicting Files
|
||
This pull request has conflicts, please resolve those before we can evaluate the pull request.
Would you mind updating the PR for me? I would really like to test it, and it would be easiest to compare if we were back up to date. :)
First testing with running npm ci - one after the other on the 6.1-dev branch and then with the PR and comparison of the output.
installer.min.css - missing version on image
installer.cssclient.css -> client.min.cssI will test it again thoroughly after the branch update.
| Labels |
Removed:
Conflicting Files
|
||
I updated the PR.
And fixed "min" version. Thanks for testing.
hm, I think there still may be an old issue with removed copyright in CSS.
I totally forgot about that.
This pull request has conflicts, please resolve those before we can evaluate the pull request.
For removed copyright we have to edit comments from /** to /*! parcel-bundler/lightningcss#43 (comment)
Can probably do it automatically, will look on it later.
| Labels |
Added:
Conflicting Files
|
||
I thought today would be a good day to test, since the beta release is coming out and there probably shouldn't be so many merges, so that the test result is quite easy to evaluate.
I'll keep testing what I can and get back to you if I find anything.
For copyright issues, please let me know if you say I can test it. Until then, I will ignore this error.
missing version in js but min.js is ok:
- media/vendor/bootstrap/js/alert.js
- media/vendor/bootstrap/js/button.js
- media/vendor/bootstrap/js/carousel.js
- media/vendor/bootstrap/js/collapse.js
- media/vendor/bootstrap/js/dropdown.js
- media/vendor/bootstrap/js/modal.js
- media/vendor/bootstrap/js/offcanvas.js
- media/vendor/bootstrap/js/popover.js
- media/vendor/bootstrap/js/scrollspy.js
- media/vendor/bootstrap/js/tab.js
- media/vendor/bootstrap/js/toast.js
media/vendor/altcha/css/altcha.css is minified, but there is no min.css.
Yes, most of files from media/vendor/ comes from npm_modules, without modification.
altcha does not provide source for these file same for js file, there a few other vendors like that.
We can duplicate these file, but that does not really make sense.
I think it is fine as long as it works.
missing version in js but min.js is ok:
need to check
For my understanding, should the media_source folder still be present in the build package?
What do you mean?
com_media It just as any other asset, but with own builder.
and where from that screenshot?
Yes, most of files from media/vendor/ comes from npm_modules, without modification.
altcha does not provide source for these file same for js file, there a few other vendors like that.
We can duplicate these file, but that does not really make sense.
I think it is fine as long as it works.
With the new build script now we have a altcha.css but it is minified, before we had only a altcha.css but it was not minified. So now, with the new script, it is no longer just copied, as it was before. Perhaps then the file should be not .css but min.css? But in the end, it's not important to me at all. I just wanted to report it because I noticed it. :D
What do you mean?
com_media It just as any other asset, but with own builder.
and where from that screenshot?
The screenshot is a direct comparison of the media assets with the previous build script and then with the new one. These are the media assets that we will deliver in the media folder in the installable package.
Until now, we have not included the source files under build/media_source in the installable package. I think this will remain the case, so media_source will not be included in the final package. But then the path doesn't fit here, because it leads to nowhere. Basically, this is tricky because moving the files means that the files for workflow-graph and mediamanager are now no longer included. Or am I mistaken?
With the new build script now we have a altcha.css but it is minified, before we had only a altcha.css but it was not minified.
It maybe changed something on their side. The file is from node_modules and it is also minified
node_modules/altcha/dist_external/altcha.css on 6.1 branch. Just checked.
com_media
Now I see, it is some random Vue thing from its compiler, the file itself is not used.
All .vue files is compiled in to javascript. We can safely ignore that.
This pull request has conflicts, please resolve those before we can evaluate the pull request.
| Labels |
Removed:
Conflicting Files
|
||
@LadySolveig I fixed the stuff you found. The license comments and bootstrap version.
@charset "UTF-8"; at the beginning of the .css and .min.css files is removed.
// eslint-disable-next-line no-undef on first line is removed in file media/vendor/jquery/js/jquery-noconflict.js
different order in css min.css - but seems ok to me, as far as I have seen it is now correct (alphabetically)
all copyrights and all comments now seem to be preserved, also on min.css and also the comments within the doucment
formatting seems to be different in css files (do we use here a different configuration???)
media/system/html/noxml.html different
-moz-radial-gradientand -webkit-radial-gradient seems missing - not sure what else
joomla.asset.json has a dfferent order,
The order should be the same for every build now. Or it still different?
However it will be different from what we currently have in main branch.
@charset "UTF-8"; at the beginning of the .css and .min.css files is removed
Hm it should be there all the time, after last fix.
Any specific css for example?
// eslint-disable-next-line no-undef
That something useless now :)
jslint is fine
media/system/html/noxml.html
Before the PR css for the template files was not processed,
Now all CSS code uses same processing routine.
different order in
The compiler and its options is the same.
Well, should be fine :)
joomla.asset.json has a dfferent order,
The order should be the same for every build now. Or it still different? However it will be different from what we currently have in main branch.
That's cool :) You are right only differs from the previous we have in the branch 👍🏼 But if a ran it now multiple times it stays the same.
Seems to be only the css and min.css files in the media/vendor folder,
Can be, the vendor files copied as they are. With few exceptions.
This pull request has conflicts, please resolve those before we can evaluate the pull request.
This pull request has been automatically rebased to 6.2-dev.
| Title |
|
||||||
Hi Fedir - thank you for your great work. This PR is one the first we would like to bring into 6.2 - could you check for the conflicts and do clean manual rebase to 6.2? I have done some tests and it worked but got some discrepancies when trying to merge in my 6.2 branch. Thank you
| Labels |
Added:
Conflicting Files
PR-6.2-dev
|
||
It is fixed
I have compared the nightly builds with and without the PR. First two findings: 1) the "version" hash-fingerprint is missing in the joomla.asset.json files. 2) installation\template\css is missing. Thanks for checking
| Labels |
Removed:
Conflicting Files
|
||
| Labels |
Removed:
PR-6.1-dev
|
||
installation\template\css is missing
I fixed the installation, and it works locally.
To fix tests it need to update ci.yaml config. I did needed changes here, but they ignored I think.
Okay, should be good now
This pull request has conflicts, please resolve those before we can evaluate the pull request.
| Labels |
Added:
Conflicting Files
|
||
| Labels |
Removed:
Conflicting Files
|
||
| Status | Pending | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2026-04-12 21:50:57 |
| Closed_By | ⇒ | MacJoom |
Thank you for the work!
When checking for deleted files and folders in 6.2-dev, I have found the following caused by this PR where it is not 100% clear to me if that is desired or not.
In the media folder we do not ship file /media/index.html anymore.
Is that expected? Or should we add it back, e.g. to make sure directory listing will not be possible in this folder?
We do not ship the unminified /media/vendor/accessibility/js/accessibility.js file anymore, only the minified and the gzipped files accessibility.min.js and accessibility.min.js.gz.
Is that expected? Or should we add the unminified version back, e.g. for debug mode?
For TinyMCE languages in folder /media/vendor/tinymce/langs we do not ship the minified and the gzipped files .min.js and .min.js.gz anymore.
Is that expected? Or should we add them back?
For TinyMCE model, plugins and themes we do not ship the index.js files anymore.
These files were not sipped with minified and not gzipped files.
They allowed to export model, plugins and themes for usage with module loaders in common JS or ES2015.
Do I assume that it's right not to ship them anymore?
You can easily verify that by comparing the full installation zip of the latest nightly builds of 6.2 and 6.1.
@LadySolveig as you were involved in the build scripts in past, and @brianteeman as you might remember things from other issues: Could you check my findings and let me know what you think? Thanks in advance.
I'm sorry, I can't help with that at the moment.
this completely untested pr should never have been merged and should be reverted (at least temproarily) so we can actually still work on 6.2 - currently impossible because it fails to work on windows
this completely untested pr should never have been merged and should be reverted (at least temproarily) so we can actually still work on 6.2 - currently impossible because it fails to work on windows
It had been open for a long time beforehand, so there had been plenty of time for testing. In my oppinion it was the right decision to merge it so early in the release cycl and not to wait any longer. Better to do it now than to leave it until the last minute and then realise the issues.
On windows, WSL can be used at the moment as a workarounds.
It had been open for a long time beforehand, so there had been plenty of time for testing. In my oppinion it was the right decision to merge it so early in the release cycl and not to wait any longer. Better to do it now than to leave it until the last minute and then realise the issues.
On windows, WSL can be used at the moment as a workaround.
just because there had been time for testing - the facts are that it was not tested and was merged anyway which is contrary to our standard operating procedures
requiring people to use WSL means asking people to rebuild all their own workflows and tooling because of a silly bug which is not acceptable - unless you deliberately want to exclude lots of people from contributing to joomla - which sadly seems to be the way this project is going
You are certainly welcome to hold a different point of view here. As long as there is a solution available for Windows, it remains the case that no one is being locked out. I cannot help with this specific problem, as I do not use Windows. However, what I know from my own experience is that it is an even worse idea to merge changes for the build scripts only shortly before an official release because you were waiting for tests.
4. For TinyMCE model, plugins and themes we do not ship the `index.js` files anymore. These files were not sipped with minified and not gzipped files. They allowed to export model, plugins and themes for usage with module loaders in common JS or ES2015. Do I assume that it's right not to ship them anymore?
Maybe @dgrammatiko can answer this question? I mean those diverse index.js with content like this in file media/vendor/tinymce/plugins/accordion/index.js:
// Exports the "accordion" plugin for usage with module loaders
// Usage:
// CommonJS:
// require('tinymce/plugins/accordion')
// ES2015:
// import 'tinymce/plugins/accordion'
require('./plugin.js');
From reading the comment I would think we don't need to ship them. But maybe we should do due to some b/c considerations?
2. We do not ship the unminified `/media/vendor/accessibility/js/accessibility.js` file anymore, only the minified and the gzipped files `accessibility.min.js` and `accessibility.min.js.gz`. Is that expected? Or should we add the unminified version back, e.g. for debug mode?
Meanwhile I've found out that the unminified file accessibility.js we ship with 6.1 (and 5.4) is not really unminified, it is already minified and so identical with accessibility.min.js.
So it could indeed make sense not to ship that accessibility.js file and to remove it on update if the web asset manager will load the minified accessibility.min.js also in debug mode when there is no unminified accessibility.js.
I don't have the time now to test that. Maybe someone else is faster.
For 2 - to original file is also minified. You can see this
In this case, the npm package itself no longer ships an unminified file in the dist directory. Feel free to verify this yourself, it therefore has absolutely nothing to do with the script here, but rather with a change in the vendor package.
For 2 - to original file is also minified. You can see this
In this case, the npm package itself no longer ships an unminified file in the dist directory. Feel free to verify this yourself, it therefore has absolutely nothing to do with the script here, but rather with a change in the vendor package.
For 2 - to original file is also minified. You can see this
In this case, the npm package itself no longer ships an unminified file in the
distdirectory. Feel free to verify this yourself, it therefore has absolutely nothing to do with the script here, but rather with a change in the vendor package.
@LadySolveig Yes, I've found out in the meantime. So the answer to question 2. is that this is desired. Thanks for checking.
just because there had been time for testing - the facts are that it was not tested
Yes, of course, the communication further up in the feed was pure small talk, and we were comparing files because we enjoy it so much. It may not have been tested on Windows, I'm not denying that.
But please, do still value the time that people here have already taken out of their schedules to test and work together to resolve potential issues before.
And please value the time of those who try to contribute but can't because of this buggy script already two of whom have taken the time to propose solutions
Please stop this discussion. It’s not helping anyone. We should be grateful that someone is working on improvements and accept that mistakes happen. (People who work make mistakes; people who test – or don’t test – make mistakes; that’s just the way it is.) In the interests of the project, we should now work together to rectify the damage.
I will commit a draft PR.
@richard67 all that is expexted,
There alao was a litle clean up in this PR.
For example min. lang files not used, index.js also not in use.
Important that the mentioned features work as before.
And please value the time of those who try to contribute but can't because of this buggy script already two of whom have taken the time to propose solutions
I understand there some issues were found, I currently far away from laptop very much. When i will have a time i will look. But it will not be fast.
@richard67 all that is expexted, There alao was a litle clean up in this PR. For example min. lang files not used, index.js also not in use. Important that the mentioned features work as before.
@Fedik Thanks for your feedback. I've meanwhile noticed also that everything I have mentioned here is as expected. The only possibly questionable thing is that the /media/vendor/codemirror/LICENSE is not present anymore. See also PR #47782 .
I’m really grateful to you for this huge improvement. I’m sure we’ll have the incompatibility with Windows sorted out soon as well.
@richard67 good catch. At first glance, it looks to me as though the license files in the CodeMirror source subdirectories are identical. Perhaps we could simply add an fs.copy into the Builder here media_source/plg_editors_codemirror/builder.mjs to pull one of the license files over. I'll take a closer look at this tomorrow.
@richard67 good catch. At first glance, it looks to me as though the license files in the CodeMirror source subdirectories are identical. Perhaps we could simply add an
fs.copyinto the Builder heremedia_source/plg_editors_codemirror/builder.mjsto pull one of the license files over. I'll take a closer look at this tomorrow.
@LadySolveig Yes, I think that's the way to go. We can just copy the file from any of the diverse CodeMirror source subdirectories to the target directory. We can either check if it already exists in the target directory and only copy if not. Or maybe easier: We just copy it every time and let them overwrite each other.
About license files, some vendors renamed them, eg from license to license.md or from .txt to something.
I updated few, but may be missed a few, or they were renamed recently.
About license files, some vendors renamed them, eg from license to license.md or from .txt to something.
I updated few, but may be missed a few, or they were renamed recently.
I have seen some new license files when I have checked, so to me that seems to be fine. Only the /media/vendor/codemirror/LICENSE is missing now, but that might be due to the special case for codemirror that we have several codemirror-something dependencies in node_modules, but we have only one target folder in media/vendor. I think we can fix that.
Found the issue, why the licence is missing. It is copied in the vendor step and cleaned up again in the builder itself. Thanks @Fedik for this missing piece, your feedback was really helpful to now where to look at. I will prepare to alternative PRs tomorrow, you can decide on.
Found the issue, why the licence is missing. It is copied in the vendor step and cleaned up again in the builder itself. Thanks @Fedik for this missing piece, your feedback was really helpful to know where to look at. I will prepare to alternative PRs tomorrow, you can decide on.
So we can't watch all assets anymore?
Yeap it was intentionally.
My idea was that the developer usualy working only on 1-2 things, no need to run whole thing every time.
Hm, no idea why Cypress need jQuery to run php 8.3 😄