theme-scripts
theme-scripts copied to clipboard
theme-sections should include dynamic import logic
Problem
In its current state, theme-sections is setup to load JS from all potential sections that show up on the home page, regardless if the merchants actually includes them or not in their layout. This means that unless the merchant uses every section available to them, we are loading in too much JS.
For example, a typical scripts/templates/index.js
might statically imports and loads JS for all potential sections, regardless if those sections actually exist on the page or not:
import sections from './slate/sections';
window.theme.sections = sections;
// import all potential sections
import './sections/featured-blog';
import './sections/featured-product';
import './sections/featured-slider';
import './sections/list-collections';
import './sections/map-section';
import './sections/newsletter';
import './sections/quotes-section';
import './sections/slideshow-section';
import './sections/video-section';
$(document).ready(function() {
templates.load('*');
sections.load('*');
});
Solution
Theme-sections should be setup in a manor which allows section JS to be dynamically imported based on if it is present on the page or not.
A basic example of what checking the page before dynamically importing the JS for a section:
async function loadSection(type
const sections = document.querySelectorAll(`[data-section-type="${type}"]`);
if (sections.length > 0) {
await import(/* webpackChunkName: type */ `section.${type}`);
return sections.load(type);
}
}
loadSection('featured-slider');
One way i currently get around this is:
getElements('[data-module]').forEach(el => {
const name = el.getAttribute('data-module')
const Module = require(`../modules/${name}`).default
new Module(el)
})
On each section i'd add a data-module="carousel-section"
and then it would dynamically load it instead of the upfront.
Would love to see this in theme-sections
🤙
@t-kelly i tried implementing this with the slate.config.js
and successfully got it creating the bundles. The only issue i had was when deployed Webpack was giving me a Cannot find module section.blah
. Is this because Slate needs work to be able to handle these and once that's done, this can be achieved?
@dan-gamble we need to be generating asset urls for each of these modules. Are you able to do that with your fix?
Having played around with your way @t-kelly i realised my "Fix" wasn't a fix at all. It all gets bundled in to 1 file and is only conditionally executed as opposed to loaded.
I did get a decent way with what you had above but i think there was some Slate stuff getting in the way. I'll have another play and see if i can recreate where i got.
Gave this another go to see if i could replicate my issue i was having @t-kelly.
So i started by expanding my slate.config.js
with:
'webpack.entrypoints': config => {
const entrypoints = {}
// Module entrypoints
fs.readdirSync(path.join(config.get('paths.theme.src.scripts'), 'modules')).forEach(file => {
const name = path.parse(file).name
const jsFile = path.join(
config.get('paths.theme.src.scripts'),
'modules',
`${name}.js`
)
if (fs.existsSync(jsFile)) {
entrypoints[`module.${name}`] = jsFile
}
})
return entrypoints
}
This works fine:
Entrypoint layout.theme = layout.theme.css.liquid layout.theme.js layout.theme.js.map
Entrypoint template.gift_card = template.gift_card.js template.gift_card.js.map
Entrypoint template.product = template.product.js template.product.js.map
Entrypoint template.addresses = template.addresses.js template.addresses.js.map
Entrypoint template.login = template.login.js template.login.js.map
Entrypoint module.collection-per-page = module.collection-per-page.js module.collection-per-page.js.map
Entrypoint module.collection-sort-by = module.collection-sort-by.js module.collection-sort-by.js.map
Entrypoint module.drawers = module.drawers.js module.drawers.js.map
Entrypoint module.faqs-section = module.faqs-section.js module.faqs-section.js.map
Entrypoint module.push-cart = module.push-cart.js module.push-cart.js.map
I then have a loadModule
function:
export async function loadModule (name) {
return await import(/* webpackChunkName: name */ `module.${name}`)
}
Used like:
getElements('[data-module]').forEach(async el => {
const name = el.getAttribute('data-module')
console.log(await loadModule(name))
})
All this seems to work fine until i get to the browser where i'm faced with:
Done a bit of googling with not much luck, don't think it's a Slate issue per-se, probably more of a webpack one but i am a bit of a webpack noob
To add to this. Changing the loadModule
function to be:
await import(/* webpackChunkName: name */ '../modules/' + name + '.js')
Does seem to work. Maybe i've just not worked how to get it to load chunks properly.
@t-kelly just curious to see if you knew any potential fixes to the above? Not wanting to accelerate this issue per-se, happy to have it in my slate.config
until you can work your magic fingers with this one 🤙
TBH I haven't had time to focus on the content of your exploration. Glancing through, your errors don't ring any bells. I need to wrap up https://github.com/Shopify/theme-scripts/pull/70. Will try and look at this next week.
Don't want to put any pressure on you at all with this @t-kelly. Just wondered if you had any ideas off the top of your head, no worries at all that you don't. Looking forward to #70 🤙