Invalid shims in Magento core break `core-bundle.js` at runtime

[EirikSl]

That is a list of loaded JS files in waterfall order.

Pay attention to the order of files core-bundle.js and jquery.cookie.js
core-bundle.js includes jquery-storageapi.js at the same time, storageapi is depended from
'jquery/jquery-storageapi': {'deps': ['jquery/jquery.cookie']}

As you see from Image jquery.cookie.js is loaded later bundle. In result, I have some Js error because of undefined Jquery methods.

Looks like all files included in the bundle should be checked for dependencies, and I afraid they should be sorted in the bundle in the correct order.

[DrewML]

Thanks for the report Victor.

It looks like there are a few things going on here.

Issue with jquery.cookie

In Magento core, the following shim config is present (as you mentioned):

'jquery/jquery-storageapi': {
   'deps': ['jquery/jquery.cookie']
}

But, jquery/jquery-storageapi is already an AMD module - shims are meant for non-AMD modules. From the RequireJS docs:

So this is a bug we need to fix in Magento core - @adifucan is working on getting this fix landed in core. For the time being, you have a few options:

1. Edit jquery/jquery-storageapi and manually add jquery/jquery.cookie to the dependency list
2. Map jquery/jquery-storageapi to a different file, where you first import jquery/jquery.cookie, then jquery/jquery-storageapi
3. Add a new requirejs-config.js in your theme that is just:

var config = {
   deps: ['jquery/jquery.cookie']
};

Loading of requirejs-config.js

I noticed that you're currently loading core-bundle.js and requirejs-config.js. This was a good guess since we don't have Magento_Baler done yet, but you'll need to make some small changes here.

When baler runs, it adds requirejs-bundle-config.js to each published theme root. You should be loading this instead of requirejs-config.js. This file is injected with configuration that tells RequireJS which modules will be in core-bundle.js, which allows core-bundle.js to be loaded asynchronously without worrying about races.

Besides that, you don't need a script tag for core-bundle.js - RequireJS will automatically fetch it. Instead, change your script tag to a preload to hint to the browser to start the download ASAP.

Other

Looks like all files included in the bundle should be checked for dependencies, and I afraid they should be sorted in the bundle in the correct order.

This is actually the way things work! All dependencies are traced in traceAMDDependencies.ts.

Then, in computeDepsForBundle.ts, we crawl the dependency graph depth-first (the same order RequireJS executes modules at runtime), and build the final module list, in order.

[EirikSl]

Thank you for such a detailed reply and sorry for not clear situation with "requirejs-config.js" - this is renamed requirejs-bundle-config.js in my project

We have shim for not only jquery/jquery-storageapi" but for "mage/common" too. I think you've already seen this in baller log.
Found shim configuration for "mage/common"...
Found shim configuration for "jquery/jquery-storageapi"....

And as you understand the wrong shim can be in requirejs-config.js from any other third-party Magento extensions and we can't override requirejs-config.js for separate extension in our child theme. So maybe fix #3 is the best for such situations.

There are 2 questions:
1 Can baler automate such fix
2 If not for #1. Will core fixes from @adifucan come to Magento 2.2?

[DrewML]

Can baler automate such fix

I think it's possible for us to auto-fix in certain situations, but I'm not quite convinced that would be the right approach. Because it's not a supported way to use the RequireJS API, we'd be building in a hack that could have unintended side-effects. Need to think on this a bit more.

Will core fixes from @adifucan come to Magento 2.2

Backports will happen for all currently supported versions of Magento. I believe there will be composer patches (or something similar) released in the interim for those that can't wait for a new release.

[friendscottn]

Baler, gives me two additional warnings about shims besides the jquery/jquery-storageapi shim.

Found shim configuration for "mage/common", but it is already an AMD module. RequireJS does not support shimming AMD modules. You may see unexpected behavior as a result.
Found shim configuration for "moment", but it is already an AMD module. RequireJS does not support shimming AMD modules. You may see unexpected behavior as a result.

They seem to stem from the same core file as the issue above.
vendor/magento/module-theme/view/base/requirejs-config.js

var config = {
   ...
    'shim': {
        ...
        'mage/common': ['jquery'],
        ...
        'moment': {
            'exports': 'moment'
        },
        ...
        'jquery/jquery-storageapi': {
            'deps': ['jquery/jquery.cookie']
        }
    }
   ...
}

I'm running against 2.2.9, but I can see the same shims in 2.3.develop too.
Are these two shims also on @adifucan 's radar to fix?

[adifucan]

Hi @friendscottn! Thanks for reporting. Those 2 shims are also eliminated in my fix. Next week it will be merged to mainline. I’ll keep you updated. Thanks!

[friendscottn]

Hey that's great @adifucan. Thank you!

[DrewML]

Added a doc on this that we now surface in the CLI: https://github.com/DrewML/baler/blob/master/docs/INVALID_SHIM.md

┌─[andrewlevine]  [~/sites/baler]
└─▪ baler
Collecting data about themes and modules from the store
Data collection completed in 145ms
Found 1 eligible theme to optimize
  - Magento/luma
Starting to bundle core-bundle for Magento/luma with 95 dependencies
One or more invalid shim configurations were found while bundling Magento/luma. See https://github.com/DrewML/baler/blob/master/docs/INVALID_SHIM.md
   - mage/common
   - moment
   - jquery/jquery-storageapi

[tmotyl]

@adifucan can you crosslink here PR/issue on Magento side where the shim are goong to be fixed?

[adifucan]

@tmotyl it's merged today into mainline within this commit: magento/magento2@db43c11

[Vinai]

@adifucan Will the PR make it into the 2.3.3 release?

[adifucan]

@Vinai unfortunately not. but it will be available in 2.3.4

[Vinai]

@adifucan Thank you for the reply, even though that’s sad news.
I think I‘ll try to put a vendor based patch for 2.3.2 and 2.3.3 up somewhere.

[tdgroot]

I created a patch gist for the jquery.cookie problem: https://gist.github.com/tdgroot/f95c398c565d9bbb83e0a650cdf67617