Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Commerce boilerplate review #73

Open
kptdobe opened this issue Jul 3, 2024 · 2 comments
Open

Commerce boilerplate review #73

kptdobe opened this issue Jul 3, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@kptdobe
Copy link

kptdobe commented Jul 3, 2024

For the bulk project, I am reviewing the loading sequence and most of the logic is inherited from this repo. The loading sequence diverged from the non-commerce boilerplate and it lost its target: focus on showing the LCP element as fast as possible.

One main issue is that projects have different pages, some requires the commerce stuff, some don't. And in the bulk case, the homepage (=most critical page) does not need the commerce stuff to show the LCP. The loading sequence is then polluted by a lot of other resources being loaded.

Recommendations:

  1. get rid of the speculationrules, modulepreload and preconnect from head.html - this is coming in the way of loading the LCP image as early as possible for the non-commerce pages
  2. get rid of commerce.js and configs.js from head.html - if something needs them, it can import them
  3. get rid of import maps - this is a developer preference, this does not help code reader introducing a different pattern for some of the files
  4. create commerce-light.js file with the minimal stuff required pre-LCP
  5. do not preload files, nor in the eager phase nor later. The "wait for LCP" is the only objectives of this phase. Anything needed by a potential LCP block can be directly loaded from the block. And then it is too late to preload anyway. This makes the loading sequence hard to debug and is anyway generally wrong to use.
  6. why are the dropins loaded immediately ? This really feels like optimisation for the product pages.
  7. of course (and I agree this is tricky), the LCP blocks need to have this notion of 2 phases loading, first being "do everything to load the potential LCP image - and avoid CLS" and in second step, load the rest that can be async if it takes longer. I think the ProductDetails is somehow trying to do this (+/- the preload of the main image which can be removed - just inject it eager loaded).
@kptdobe kptdobe added the enhancement New feature or request label Jul 3, 2024
@keepthebyte
Copy link

Thanks for the review @kptdobe - just one note - the speculationrules have no impact on LCP - they are just declarative for preload rules for Chrome/Edge - and don't load anything..

@fnhipster
Copy link
Collaborator

  • Drop-ins require `importmaps' to map externalized bundled dependencies such as UI components, fetch-graphql and event bus, preact, etc.
  • Drop-ins initialization has been optimized in the develop branch and they are now initialized when referenced by a Block.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants