userfrosting / UserFrosting

Modern PHP user login and management framework
https://www.userfrosting.com
Other
1.64k stars 366 forks source link

'merge' rule for bundle config files doesn't respect Sprinkle order #660

Closed alexweissman closed 6 years ago

alexweissman commented 7 years ago

When using the merge option for onCollision as documented in https://learn.userfrosting.com/building-pages/assets/asset-bundles#extending-and-overriding-bundles, the order of the assets should reflect the order of loaded Sprinkles. For example, if I have the js/form-widgets bundle in core defined as:

        "js/form-widgets": {
            "scripts": [
                "vendor/speakingurl/speakingurl.min.js",
                "local/core/js/uf-collection.js"
            ],
            "options": {
                "result": {
                    "type": {
                        "scripts": "plain"
                    }
                }
            }
        }

and then I later load a Sprinkle site with the following definition:

    "js/form-widgets" : {
        "scripts": [
            "vendor/MathJax/MathJax.js"
        ],
        "options": {
            "result": {
                "type": {
                  "scripts": "plain"
                }
            },
            "sprinkle": {
                "onCollision": "merge"
            }
        }
    }

The result is:

        "js/form-widgets": {
            "scripts": [
                "vendor/MathJax/MathJax.js",
                "vendor/speakingurl/speakingurl.min.js",
                "local/core/js/uf-collection.js"
            ],
            "options": {
                "result": {
                    "type": {
                        "scripts": "plain"
                    }
                }
            }
        }

So, the bundle-build task is putting the assets from site ahead of the assets from core. The problem only seems to happen with compiled assets - raw assets are loaded in the correct order.

Silic0nS0ldier commented 7 years ago

Based purely on observation, I'd say its sorting the array in alphabetical order. Presumably the plugin used is doing this to improve performance. A fix would entail finding a plugin that does what we want (will take awhile), or creating our own (better put off for the planned asset overhaul).

alexweissman commented 7 years ago

If it were sorting alphabetically, though, why would the vendor/ paths come first?

Silic0nS0ldier commented 7 years ago

Right...

Silic0nS0ldier commented 6 years ago

Replicated the issue. I know whats happening. Instead of appending, its prepending. ...and looking at the code, this is really obvious. Really should have named that rule append...

dev was unaffected as the PHP side handles that, and does the merging in a drastically different way.

lcharette commented 6 years ago

I can confirm that solved the issue with my Sprinkle 👍