Joomla! Issue Tracker | Joomla! CMS #44674 - [5.2][bug] Codemirror duplicated assets entries

RTC bug PR-5.2-dev

Fixed in Code Base
9 Jan 2025
Medium
Build: 5.2-dev
# 44674
Diff
dgrammatiko:patch-1

Pending

User tests: Successful: Unsuccessful:

dgrammatiko
28 Dec 2024

Redo of #44671 .

Summary of Changes

(reported by Hannes)
The file media/plg_editors_codemirror/joomla.asset.json has duplicate entries.
This PR makes sure that these entries are unique

Testing Instructions

Pre-conditions

It needs development environment with PHP CLI, composer and npm on Linux or on Windows with WSL, and a git clone of either the CMS repository or your fork of that.

Open a command shell window (Terminal on Linux, CMD with WSL on Windows) and change to the root directory of your git clone.

All commands mentioned in the test are executed in that command shell window.

It is not enough just to apply this PR. It needs to have a branch with the change so it will also be available in the tag used for building the packages.

Therefore please follow strictly the testing instructions.

Test 1: Build on clean, current 5.2-dev branch

Make sure that you have checked out the 5.2-dev branch.

git checkout 5.2-dev

If you have done some work on that branch before, like e.g. running composer or npm or making an installation, make sure to clean up the current branch and revert all local changes.

git clean -d -x -f
git checkout .

Create a new tag on the local clone.

git tag -a 5.2.3 -m "test 5.2.3"

Run the build script without any parameters so the previously created new tag will be used.

php ./build/build.php

The created packages can be found in directory ./build/tmp/packages.

Save the full installation ZIP package outside of the git clone for later comparison with the result of the other tests.
E.g. if the parent folder is writable:

cp ./build/tmp/packages/Joomla_5.2.3-dev-Development-Full_Package.zip ../Joomla_5.2.3-dev-Development-Full_Package_test-1.zip

Clean up for later tests: Delete the previously created tag.

git tag -d "5.2.3"

Test 2: Build on 5.2-dev branch after `composer install` and `npm ci`

Make sure to clean up the current branch and revert all local changes.

git clean -d -x -f
git checkout .

Run composer and npm.

composer install
npm ci

Create a new tag on the local clone.

git tag -a 5.2.3 -m "test 5.2.3"

Run the build script without any parameters so the previously created new tag will be used.

php ./build/build.php

The created packages can be found in directory ./build/tmp/packages.

Save the full installation ZIP package outside of the git clone for later comparison with the result of the other tests.
E.g. if the parent folder is writable:

cp ./build/tmp/packages/Joomla_5.2.3-dev-Development-Full_Package.zip ../Joomla_5.2.3-dev-Development-Full_Package_test-2.zip

Clean up for later tests: Delete the previously created tag.

git tag -d "5.2.3"

Compare the file media/plg_editors_codemirror/joomla.asset.json from the package created in this test with the file from the package created in the previous test "Test 1".

Result: See section "Actual result BEFORE applying this Pull Request" below the testing instructions.

Test 3: Build on clean branch of this PR

Make sure to clean up the current branch and revert all local changes.

git clean -d -x -f
git checkout .

Fetch this pull request (PR) into a new local branch and check out that branch.

git fetch upstream pull/44674/head:test-pr-44674
git checkout test-pr-44674

Create a new tag on the local clone.

git tag -a 5.2.3 -m "test 5.2.3"

Run the build script without any parameters so the previously created new tag will be used.

php ./build/build.php

The created packages can be found in directory ./build/tmp/packages.

Save the full installation ZIP package outside of the git clone for later comparison with the result of the other tests.
E.g. if the parent folder is writable:

cp ./build/tmp/packages/Joomla_5.2.3-dev-Development-Full_Package.zip ../Joomla_5.2.3-dev-Development-Full_Package_test-3.zip

Clean up for later tests: Delete the previously created tag.

git tag -d "5.2.3"

Compare the file media/plg_editors_codemirror/joomla.asset.json from the package created in this test with the file from the package created in the first test 1.

Result: See section "Expected result AFTER applying this Pull Request" below the testing instructions.

Test 4: Build on branch of this PR after `composer install` and `npm ci`

Make sure to clean up the current branch and revert all local changes.

git clean -d -x -f
git checkout .

Only if you haven't done this before e.g. when executing test 3:
Fetch this pull request (PR) into a new local branch and check out that branch.

git fetch upstream pull/44674/head:test-pr-44674
git checkout test-pr-44674

Run composer and npm.

composer install
npm ci

Create a new tag on the local clone.

git tag -a 5.2.3 -m "test 5.2.3"

Run the build script without any parameters so the previously created new tag will be used.

php ./build/build.php

The created packages can be found in directory ./build/tmp/packages.

Save the full installation ZIP package outside of the git clone for later comparison with the result of the other tests.
E.g. if the parent folder is writable:

cp ./build/tmp/packages/Joomla_5.2.3-dev-Development-Full_Package.zip ../Joomla_5.2.3-dev-Development-Full_Package_test-4.zip

Clean up for later tests: Delete the previously created tag.

git tag -d "5.2.3"

Compare the file media/plg_editors_codemirror/joomla.asset.json from the package created in this test with the file from the package created in the second test "Text 2".

Compare the file media/plg_editors_codemirror/joomla.asset.json from the package created in this test with the file from the package created in the previous test "Text 3".

Result: See section "Expected result AFTER applying this Pull Request" below the testing instructions.

Actual result BEFORE applying this Pull Request

When creating a new tag and building packages for that tag after having executed composer installand npm ci (or npm ior npm install), file media/plg_editors_codemirror/joomla.asset.json contains duplicate script assets and dependencies.

On a clean branch without having executed composer installand npm ci this does not happen.

Besides this, the order of assets in the mentioned file or any other joomla.asset.json file might be different to the result from a build on a clean branch.

Expected result AFTER applying this Pull Request

Creating a new tag and building packages for that tag on a clean branch, i.e. no previous composer or npm run, works with this PR applied as well as without.

When having run composer or npm run before that, the issue does not happen anymore with this PR applied.

The order of assets in the mentioned file or any other joomla.asset.json file might be different to the result from a build on a clean branch.
This could maybe be fixed with another PR.

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

@richard67

4ae7e36 28 Dec 2024

Update build-codemirror.es6.js

dgrammatiko - open - 28 Dec 2024

dgrammatiko - change - 28 Dec 2024

Status

New

⇒

Pending

joomla-cms-bot - change - 28 Dec 2024

Category

⇒

JavaScript Repository

baff145 28 Dec 2024

Update build-codemirror.es6.js

dgrammatiko - change - 28 Dec 2024

Labels

Added: PR-5.2-dev

d2375ec 28 Dec 2024

Update build-codemirror.es6.js

richard67 - comment - 28 Dec 2024

@dgrammatiko For reproducing the issue it is not enough to follow the current testing instructions. In fact it's a bit more complicated. It needs to run composer install and npm cion the clean, local branch, then create a new git tag on that local branch and then run php ./build/build.php without any parameters so the latest local tag is used.

I will see if I can provide detailed testing instructions, but it might be tomorrow.

2152bf2 28 Dec 2024

Update build-codemirror.es6.js

dgrammatiko - comment - 28 Dec 2024

Could you do me favor? When testing this could you switch to 5.3, add a "type": "module", line in the package.json and then test if that happens there? I'm guessing it's the module resolution algo from NodeJS

richard67 - comment - 28 Dec 2024

Could you do me favor? When testing this could you switch to 5.3, add a "type": "module", line in the package.json and then test if that happens there? I'm guessing it's the module resolution algo from NodeJS

@dgrammatiko You mean with the changes of this PR here included? Or you mean on a clean 5.3-dev? And where exactly should I add the "type": "module", line?

dgrammatiko - comment - 28 Dec 2024

Or you mean on a clean 5.3-dev? And where exactly should I add the "type": "module", line?

on a clean 5.3 and the "type": "module", could be inserted as the first entry at the top of the package.json. fwiw the resolution algo is different for cjs and esm, so maybe we're lucky and 5.3 already solves this with a simple line in the package.json...

richard67 - comment - 28 Dec 2024

~~Hmm, I've just tested this PR, and now the gz files are not created at all. I will continue to test tomorrow.~~

Update: False alarm. I've checked the wrong media folder when comparing.

richard67 - change - 29 Dec 2024

The description was changed

richard67 - edited - 29 Dec 2024

richard67 - change - 29 Dec 2024

The description was changed

richard67 - edited - 29 Dec 2024

richard67 - comment - 29 Dec 2024

@dgrammatiko I've allowed myself to provide testing instructions so it's clear how the issue can be reproduced.

richard67 - test_item - 29 Dec 2024 - Tested successfully

richard67 - comment - 29 Dec 2024

I have tested this item ✅ successfully on 2152bf2

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44674.}

Fedik - comment - 30 Dec 2024

Interesting, so it is actually /nodel_modules gets duplicated while doing all that.
Thanks for investigation.

@dgrammatiko please just update

joomla-cms/build/build-modules-js/init/common/resolve-package.es6.js

Line 29 in eac7fd7

module.exports.getPackagesUnderScope = (scope) => {

To use Set():

module.exports.getPackagesUnderScope = (scope) => {
  const cmModules = new Set();

  // Get the scope roots
  const roots = [];
  module.paths.forEach((path) => {
    const fullPath = `${path}/${scope}`;
    if (existsSync(fullPath)) {
      roots.push(fullPath);
    }
  });

  // List of modules
  roots.forEach((rootPath) => {
    readdirSync(rootPath).forEach((subModule) => {
      cmModules.add(`${scope}/${subModule}`);
    });
  });

  return [...cmModules];
};