Removed scriptaculous

[luigifab]

It's a test. Is OpenMage works without scriptaculous? Answer: almost. Okay, I cheated, I added prototype/effects.js...

Two errors:

• Ajax.Autocompleter is undefined
• Sortable.create is undefined (js/mage/adminhtml/product.js)

[lgtm-com[bot]]

This pull request fixes 18 alerts when merging bf52078c3f4f2330cdf6027a57b796462c908f68 into 6571d9a6a305240aec9b6e472dfe5a36936cd997 - view on LGTM.com

fixed alerts:

• 7 for Missing variable declaration
• 3 for With statement
• 3 for Unused variable, import, function or class
• 2 for Overwritten property
• 1 for Useless assignment to local variable
• 1 for Useless conditional
• 1 for Variable not declared before use

[fballiano]

that would be a dream!

[addison74]

What is the role of the scriptaculos library in Magento because I see a lot of files in its directory? I visited the website but the project is no longer maintained and the links can no longer be accessed.

If its facilities can be obtained with other libraries then it can take its eternal rest to the landfill where other JavaScript libraries have arrived.

[fballiano]

@ADDISON74 it was a graphic/animation library for "prototype"

[kiatng]

This definitely breaks BC, I happened to use both variants of autocompleters in Scriptaculous in a few projects. Even though Scriptaculous is a dinouser, I am ambivalent about its removal. It's part of the core third-party libraries, along with prototype.js and validation.js.

[addison74]

I tend to agree with @kiatng. Even it is a relic in the OM code if an installed extension in production uses it then we will create immediate issues that need to be fixed. A few months ago the colorpicker library was removed by mistake but which was immediately put back. I would not have found this problem if I had not used a 3rd party extension.

[fballiano]

if we could never do BC at all we would still support php5.6, we have to move on sometimes. scriptacoulus isn't maintainer since years and years and should be removed, it's a monster from the past. maybe in 20.x or 21.x but it will never stay there forever.

the 19.x is exactly done for that reason.

[Flyingmana]

There are different kinds of BC breaks. They come with different reach, documentation, tooling, polifills.

And for a theme, the framework does support modifying themes, or even copying them into a separate one. There should be a very good reason to accept a BC break, if there is an alternative which enables to do it without a BC break. I see good reasons as:

• Does not work anymore
• Is a proven security issue
• Using it is considered harmful (with explanation how it harms the user)

[fballiano]

I think @luigifab with this PR is exactly trying to achieve a solution to that.

[luigifab]

You are right @fballiano. Moreover instead of full removing, perhaps I can simply add a Yes/No configuration in backend to disable scriptaculous.

[addison74]

My opinion that there is no justification to disable this library in OM Backend. I could say that certain files will no longer be uploaded, but they will not influence the time load much. In addition prototype and scriptaculos over time have been considered symbiotes. If you want to give up one it would be better to give up both. That's why in Magento 2 they chose JQuery, although I don't like it and I'm a veteran of vanilla JavaScript.

I propose to keep in pending this PR. It is worth appreciating the tireless work of @luigifab who for some time has been trying to clean the code of all relics.

[lgtm-com[bot]]

This pull request fixes 18 alerts when merging d129f1be7cebb9e8f1d7c5b6a1c38a80dd77b2f8 into 022992773b4479c63f87ae90b5361efecc3cec87 - view on LGTM.com

fixed alerts:

• 7 for Missing variable declaration
• 3 for With statement
• 3 for Unused variable, import, function or class
• 2 for Overwritten property
• 1 for Useless assignment to local variable
• 1 for Useless conditional
• 1 for Variable not declared before use

[luigifab]

In production for > 9 months. But yes, we are not using autocomplete, and for Sortable.create we are not adding product attributes from product edit page.

[lgtm-com[bot]]

This pull request fixes 18 alerts when merging dac5350ef1b63a8a0a06106b5481e5a7e49d481a into ae2520cf986ea6d6cdc7a0c03065aabfeb8e164b - view on LGTM.com

fixed alerts:

• 7 for Missing variable declaration
• 3 for With statement
• 3 for Unused variable, import, function or class
• 2 for Overwritten property
• 1 for Useless assignment to local variable
• 1 for Useless conditional
• 1 for Variable not declared before use

[lgtm-com[bot]]

This pull request fixes 18 alerts when merging f87423b6ab1cc319ab2934d84263a9ad72a31522 into f93c9e05db069746071903891e93dd8e515763b8 - view on LGTM.com

fixed alerts:

• 7 for Missing variable declaration
• 3 for With statement
• 3 for Unused variable, import, function or class
• 2 for Overwritten property
• 1 for Useless assignment to local variable
• 1 for Useless conditional
• 1 for Variable not declared before use

[lgtm-com[bot]]

This pull request fixes 18 alerts when merging de65b0e9ec3d577cc421cc741e508de2f9455334 into cf82b8f623e754e63c9a64dbf3c21adaf46ddc56 - view on LGTM.com

fixed alerts:

• 7 for Missing variable declaration
• 3 for With statement
• 3 for Unused variable, import, function or class
• 2 for Overwritten property
• 1 for Useless assignment to local variable
• 1 for Useless conditional
• 1 for Variable not declared before use

[lgtm-com[bot]]

This pull request fixes 18 alerts when merging cab677583b9d57a2c25f8013b52abd9ea5c19271 into bc4b2cdabf56ce3f5e78fb55df4c9b4b7c9ed105 - view on LGTM.com

fixed alerts:

• 7 for Missing variable declaration
• 3 for With statement
• 3 for Unused variable, import, function or class
• 2 for Overwritten property
• 1 for Useless assignment to local variable
• 1 for Useless conditional
• 1 for Variable not declared before use

[luigifab]

I think @loekvangool will like this PR. For me without the two errors, yes it still works :).

[loekvangool]

Haha getting a reputation, am I?

I will re-state something I said on another issue today. Probably the best course of action with these old libs in the short term is to make it possible to exclude a lib like Scriptaculous from local.xml, for all or a subset of pages, without more errors showing up than necessary. Thus, it will not break BC and in case you care about Web Vitals and you know you don't use lib xyz.js on catalog_category_view, you can exclude it using local.xml and nothing in js/varien/* will complain about it.

In general, CSS and JS code is added too liberally to every page by both core OM and 3rd party modules. Unused CSS/JS is a big deal in Web Vitals. It has to be split into more files and used where needed. More files is not bad in HTTP/2, by the way.

[fballiano]

I've rebased to next which is where this PR should be, since it is a BC.

I think we really should implement this PR and get rid of this library.

But for sure we've to reimplement Ajax.Autocompleter and the Sortable.

[fballiano]

@luigifab what is prototype/effects.js doing?

[luigifab]

A lot of things, for example there is new Effect.Appear(advice, {duration : 1}); to add an animation when displaying an error in a form. We can't remove it, but perhaps we can create a blank JS that do nothing to not break the world, with for example:

/**
 * Add classes to specified elements.
 * @deprecated
 */
function decorateGeneric() {
    console.warn('decorateGeneric is deprecated');
}

/**
 * Decorate table rows and cells, tbody etc
 * @deprecated
 */
function decorateTable() {
    console.warn('decorateTable is deprecated');
}

/**
 * Set "odd", "even" and "last" CSS classes for list items
 * @deprecated
 */
function decorateList() {
    console.warn('decorateList is deprecated');
}

/**
 * Set "odd", "even" and "last" CSS classes for list items
 * @deprecated
 */
function decorateDataList() {
    console.warn('decorateDataList is deprecated');
}

but inside effects.js and for effect functions.

Closed because this PR is incomplete... and I will be out there until 2025. Anyway, I continue use it as is.