Invalid shims in Magento core break `core-bundle.js` at runtime
EirikSl opened this issue · 13 comments
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.
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:
- Edit
jquery/jquery-storageapi
and manually addjquery/jquery.cookie
to the dependency list - Map
jquery/jquery-storageapi
to a different file, where you first importjquery/jquery.cookie
, thenjquery/jquery-storageapi
- 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.
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?
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.
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?
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!
Hey that's great @adifucan. Thank you!
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
@adifucan can you crosslink here PR/issue on Magento side where the shim are goong to be fixed?
@tmotyl it's merged today into mainline within this commit: magento/magento2@db43c11
@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.
I created a patch gist for the jquery.cookie
problem: https://gist.github.com/tdgroot/f95c398c565d9bbb83e0a650cdf67617