? NPM Resource Changed ? Maintainers Checked Pending

User tests: Successful: Unsuccessful:

avatar heelc29
heelc29
27 Aug 2022

Summary of Changes

Convert unnamed function to arrow function

Testing Instructions

follow #38154

Actual result BEFORE applying this Pull Request

warning at npm run lint:js
image

Expected result AFTER applying this Pull Request

no warning at npm run lint:js
image

avatar heelc29 heelc29 - open - 27 Aug 2022
avatar heelc29 heelc29 - change - 27 Aug 2022
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 27 Aug 2022
Category JavaScript Repository NPM Change
avatar heelc29 heelc29 - change - 27 Aug 2022
The description was changed
avatar heelc29 heelc29 - edited - 27 Aug 2022
avatar brianteeman
brianteeman - comment - 27 Aug 2022

obvioously there is an issue with drone as this should have been picked up

avatar heelc29
heelc29 - comment - 27 Aug 2022

obvioously there is an issue with drone as this should have been picked up

That it's just a warning, I guess it's ok for drone ...
image

avatar richard67
richard67 - comment - 27 Aug 2022

I have tested this item successfully on dd0e85e

Warning from eslint is fixed.


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

avatar richard67 richard67 - test_item - 27 Aug 2022 - Tested successfully
avatar viocassel
viocassel - comment - 27 Aug 2022

I have tested this item successfully on dd0e85e


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

avatar viocassel viocassel - test_item - 27 Aug 2022 - Tested successfully
avatar richard67 richard67 - change - 27 Aug 2022
Status Pending Ready to Commit
Labels Added: NPM Resource Changed ? Maintainers Checked
avatar richard67
richard67 - comment - 27 Aug 2022

RTC


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

avatar dgrammatiko
dgrammatiko - comment - 28 Aug 2022

Although this PR cleans up the warning there are more problems in this file, eg the addEventListener('DOMContentLoaded', ...) is an ANTIPATTERN and should be removed (the file is either loaded with attribute defer or type=module which render the waiting for the DOM to be ready useless). I had fixed it correctly with #38225: https://github.com/joomla/joomla-cms/pull/38225/files#diff-4afb4f910136391957e62b8afcb5f0d6b240ee226cd69b4960777bc15ddfe668

The code should be:

/**
 * @copyright  (C) 2022 Open Source Matters, Inc. <https://www.joomla.org>
 * @license    GNU General Public License version 2 or later; see LICENSE.txt
 */
[].slice.call(document.querySelectorAll('.article-status')).forEach((element) => {
  element.addEventListener('click', (event) => {
    event.stopPropagation();
  });
});

or even more compact:

/**
 * @copyright  (C) 2022 Open Source Matters, Inc. <https://www.joomla.org>
 * @license    GNU General Public License version 2 or later; see LICENSE.txt
 */
[].slice.call(document.querySelectorAll('.article-status')).forEach((element) => element.addEventListener('click', (event) => event.stopPropagation()));

@obuisard this might be a good project for 4.3: clean all the addEventListener('DOMContentLoaded', ...) from the javascript files!
The anti pattern is explained here: https://www.youtube.com/watch?v=_iq1fPjeqMQ

avatar obuisard
obuisard - comment - 28 Aug 2022

I agree @dgrammatiko, this should be cleaned up. Do you mind writing a PR for this issue and if you can't work on it I will try and rally the troops so you get help :-)

avatar dgrammatiko
dgrammatiko - comment - 28 Aug 2022

@obuisard you could use #35725 as a guide.
In short the scripts need to:

  • be converted to type=module + add the -es5 in the dependencies
  • remove the iife, if exists (type= module scripts are scoped by default)
  • remove strict mode (type= module is strict by default)
  • remove the addEventListener('DOMContentLoaded', ...) as it's pure overhead
avatar wilsonge wilsonge - close - 2 Sep 2022
avatar wilsonge wilsonge - merge - 2 Sep 2022
avatar wilsonge wilsonge - change - 2 Sep 2022
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2022-09-02 09:13:21
Closed_By wilsonge
Labels Added: ?

Add a Comment

Login with GitHub to post a comment