PR-5.2-dev Pending

User tests: Successful: Unsuccessful:

avatar dgrammatiko
dgrammatiko
13 Jul 2024

Pull Request for Issue # .

Summary of Changes

  • Move tools and source JS to ESM
  • ES6 files have now the extension .mjs (used to be .es6.js)
  • Legacy js files now have a .js extension instead of .es5.js
  • Web Components also have .mjs extension (used to be .w-c.es6.js)
  • The build/media_source/system/js/joomla-core-loader.mjs file now includes the required css and the related build/media_source/system/scss/joomla-core-loader.scss is deleted

Testing Instructions

You need Git and NPM to test this

Pull the 4.4-dev branch and run:

  • npm install
  • npm run cssversioning
  • npm run gzip
  • npm run versioning
  • Store the media folder somewhere outside of the joomla-cms folder

Pull this PR gh pr checkout 43779 and run:

  • npm install
  • npm run cssversioning
  • npm run gzip
  • npm run versioning
  • Store the media folder somewhere outside of the joomla-cms folder

Compare the two folders. The only difference should be the missing files: media/system/css/joomla-core-loader.css, media/system/css/joomla-core-loader.min.css, media/system/css/joomla-core-loader.min.css.gz

Check the basic backend functionality

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@Fedik @richard67 could you please test this one (pretty hard for the average tester)?

avatar dgrammatiko dgrammatiko - open - 13 Jul 2024
avatar dgrammatiko dgrammatiko - change - 13 Jul 2024
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 13 Jul 2024
Category JavaScript Administration com_media NPM Change Repository
avatar dgrammatiko dgrammatiko - change - 13 Jul 2024
Labels Added: NPM Resource Changed PR-5.1-dev
avatar dgrammatiko dgrammatiko - change - 13 Jul 2024
The description was changed
avatar dgrammatiko dgrammatiko - edited - 13 Jul 2024
avatar joomla-cms-bot joomla-cms-bot - change - 13 Jul 2024
Category JavaScript Administration com_media NPM Change Repository JavaScript Administration com_media NPM Change
avatar Fedik
Fedik - comment - 13 Jul 2024

image

avatar dgrammatiko
dgrammatiko - comment - 13 Jul 2024

Hey, at least there are clear test instructions ?

avatar Fedik
Fedik - comment - 16 Jul 2024

I would keep settings.json as it is now. To avoid bloating of package.json.

Other than that, the PR seems working, node comands still working.

avatar dgrammatiko
dgrammatiko - comment - 16 Jul 2024

I would keep settings.json as it is now. To avoid bloating of package.json.

It can be reverted, although personally I see that the settings in the package is easier than searching for some file that all the settings are stored.
Anyways not too strong opinion here, I would revert it

avatar Fedik
Fedik - comment - 16 Jul 2024

Yes please.
It is better to keep them apart, there mostly joomla stuff, nothing standart for package.json

And it really huge.

avatar dgrammatiko
dgrammatiko - comment - 16 Jul 2024

Ok, done

avatar Fedik Fedik - test_item - 17 Jul 2024 - Tested successfully
avatar Fedik
Fedik - comment - 17 Jul 2024

I have tested this item ✅ successfully on 46da720


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

avatar C-Lodder
C-Lodder - comment - 19 Jul 2024

Why not just use "type": "module" in the package.json instead of using .mjs files?

avatar dgrammatiko
dgrammatiko - comment - 19 Jul 2024

instead of using .mjs files

Couple reasons:

  • existing common js files will need to be renamed to .cjs
  • most important, this simplifies the whole thing: .mjs modern js, .js legacy, iife staff
avatar C-Lodder
C-Lodder - comment - 19 Jul 2024

Oh you still have CJS. Fair enough.
I don't understanding mixing .cjs, .mjs and .js in 2024, as Node.js has supported ES Modules for ages.

avatar dgrammatiko
dgrammatiko - comment - 22 Jul 2024

I don't understanding mixing .cjs, .mjs and .js in 2024, as Node.js has supported ES Modules for ages.

Actually what I'm doing here is using the mjs/js extensions as a convention for the tools.

Could it be done with just the js extension?
Sure by using some prefix like we did .es6.js but since these are the source files (not deliverables) it's up to the maintainers to choose their poison (the point is that I've eliminated one case here the .w-c.js, now covered by .mjs) a 3 letter extension (mjs) or a 6 one (es6.js). FWIW .mjs is totally supported by node (actually was invented for cases where there were more than ESM files, ie current Joomla's case)...

avatar dgrammatiko
dgrammatiko - comment - 27 Jul 2024

Is there any interest on this one or should I close it?

avatar Fedik
Fedik - comment - 27 Jul 2024

or should I close it?

hold on, not so fast ?

avatar bembelimen
bembelimen - comment - 11 Aug 2024

This should be something in Joomla! but sorry, not in a Bugfix release. Moved it to 5.2.

avatar joomla-cms-bot joomla-cms-bot - change - 11 Aug 2024
Category JavaScript Administration com_media NPM Change Repository JavaScript
avatar dgrammatiko
dgrammatiko - comment - 11 Aug 2024

Ok, I reduced the changes only to the tools.
The source folder needs coordination as any conflicts are painful to be fixed. I'm ok with doing the work if the maintainers accept the changes and coordinate the RL so I don't have to fight unmerges etc...

avatar Quy Quy - change - 11 Aug 2024
Title
[5.1] Move to ESM
[5.2] Move to ESM
avatar Quy Quy - edited - 11 Aug 2024
avatar dgrammatiko dgrammatiko - close - 12 Aug 2024
avatar dgrammatiko dgrammatiko - change - 12 Aug 2024
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2024-08-12 12:28:07
Closed_By dgrammatiko
Labels Added: PR-5.2-dev
Removed: NPM Resource Changed PR-5.1-dev
avatar dgrammatiko dgrammatiko - change - 12 Aug 2024
Status Closed New
Closed_Date 2024-08-12 12:28:07
Closed_By dgrammatiko
avatar dgrammatiko dgrammatiko - change - 12 Aug 2024
Status New Pending
avatar dgrammatiko dgrammatiko - reopen - 12 Aug 2024
avatar Fedik
Fedik - comment - 12 Aug 2024

@dgrammatiko please reset the changes in settings.json, as I see it is only formating, I do not realy want it to affect the history.
Thanks!

Add a Comment

Login with GitHub to post a comment