Refactoring Bacom utils.js / scripts.js for performance and consuming blocks using getLibs

[JasonHowellSlavin]

Hey folks, I had a question regarding what we might consider best practice for consuming repositories. In an attempt to help reduce the amount of calls, bacom is moving the getLibs/setLibs function from a separate consumer utils.js file to our scripts.js file, which will allow the code to omit a render blocking network request and improve our LCP.

However, as a consumer we have a few blocks that call getLibs to get the correct path for utilities like createTag. Previously, when we would import setLibs into our scripts.js and call setLibs, we would set the module scope with the correct libs path, allowing our consumer blocks to import the correct getLibs from the module scope. By removing this import and not exporting a set/get, we've broken consumer blocks that rely at that module scope.

Since this is an issue that may be faced by other consuming projects, we thought it best to bring it up in a community discussion. The paths forward as I can see them are as follows:

• Export get/setLibs from scripts.js to expose module scope to consumer blocks and update import paths
  • Pros: little effect on the blocks, and similar to current utils.js functionality. Able to delete consumer utils.js for now
  • Cons: not sure exporting from scripts is a pattern we want to establish
• Call setLibs with our libs param
  • Pros: allows code to stay in utils.js
  • Cons: requires us to either export our LIBS const, or call with './libs' since we are aware we want Milo libs. Will also create the same utils call dependency for non milo blocks.
• Set the milolibs path to a global, like window.libsPath
  • Pros: Makes the libspath available without function call
  • Cons: Pollutes the global scope

I think exporting from scripts will be the cleanest, but wanted to see what the community thought. Thank you all for your feedback!

[auniverseaway]

We have evolved some of our patterns since we established consumer utils back in 2022. We've learned a lot about side effects and how to use them to our advantage.

1. I think we can get rid of set / get and only do get. Get is probably not even necessary.
2. I'd keep utils around for non-critical / non-lcp functions, but only move libs into scripts.js.
3. I consider using a window object in 2024 as a code smell. There's no reason to do this with ESM.
4. You're 100% right that we don't like to export out of scripts, but it's probably ok for libs.

This is pretty close to what I would put in scripts.js...

export const LIBS = (() => {
  const { hostname, search } = window.location;
  if (!(hostname.includes('.hlx.') || hostname.includes('local'))) return '/libs';
  const branch = new URLSearchParams(search).get('milolibs') || 'main';
  if (branch === 'local') return 'http://localhost:6456/libs';
  return branch.includes('--') ? `https://${branch}.hlx.live/libs` : `https://${branch}--milo--adobecom.hlx.live/libs`;
})();

[JasonHowellSlavin]

Thank you @auniverseaway! This lines up very closely with the code we have in our current branch for this fix. We will go ahead with this implementation.

Would you want me to create an issue or raise a PR against milo-college to modify the libs function there to reflect this change?

[Brandon32]

@auniverseaway If we keep setLibs as a function then we can unit test on the location.

export function setLibs(location) {
  const { hostname, search } = location;
  if (!['.hlx.', '.stage.', 'local'].some((i) => hostname.includes(i))) return '/libs';
  const branch = new URLSearchParams(search).get('milolibs') || 'main';
  if (branch === 'local') return 'http://localhost:6456/libs';
  return branch.includes('--') ? `https://${branch}.hlx.live/libs` : `https://${branch}--milo--adobecom.hlx.live/libs`;
}

export const LIBS = setLibs(window.location);

This would be useful to make sure any future changes do not break production.

import { expect } from '@esm-bundle/chai';
import { setLibs } from '../../scripts/scripts.js';

describe('Libs', () => {
  const tests = [
    ['https://business.adobe.com', '/libs'],
    ['https://business.adobe.com?milolibs=foo', '/libs'],
    ['https://business.stage.adobe.com', 'https://main--milo--adobecom.hlx.live/libs'],
    ['https://business.stage.adobe.com?milolibs=foo', 'https://foo--milo--adobecom.hlx.live/libs'],
    ['https://business.stage.adobe.com?milolibs=awesome--milo--forkedowner', 'https://awesome--milo--forkedowner.hlx.live/libs'],
    ['https://main--bacom--adobecom.hlx.page/', 'https://main--milo--adobecom.hlx.live/libs'],
    ['https://main--bacom--adobecom.hlx.page/?milolibs=foo', 'https://foo--milo--adobecom.hlx.live/libs'],
    ['https://main--bacom--adobecom.hlx.page/?milolibs=local', 'http://localhost:6456/libs'],
    ['https://main--bacom--adobecom.hlx.page/?milolibs=awesome--milo--forkedowner', 'https://awesome--milo--forkedowner.hlx.live/libs'],
    ['https://main--bacom--adobecom.hlx.live/', 'https://main--milo--adobecom.hlx.live/libs'],
    ['https://main--bacom--adobecom.hlx.live/?milolibs=foo', 'https://foo--milo--adobecom.hlx.live/libs'],
    ['https://main--bacom--adobecom.hlx.live/?milolibs=local', 'http://localhost:6456/libs'],
    ['https://main--bacom--adobecom.hlx.live/?milolibs=awesome--milo--forkedowner', 'https://awesome--milo--forkedowner.hlx.live/libs'],
    ['http://localhost:3000', 'https://main--milo--adobecom.hlx.live/libs'],
    ['http://localhost:3000?milolibs=foo', 'https://foo--milo--adobecom.hlx.live/libs'],
    ['http://localhost:3000?milolibs=local', 'http://localhost:6456/libs'],
    ['http://localhost:3000?milolibs=awesome--milo--forkedowner', 'https://awesome--milo--forkedowner.hlx.live/libs'],
  ];

  tests.forEach(([url, expected]) => {
    it(`Sets libs for ${url}`, () => {
      const location = new URL(url);
      const libs = setLibs(location);
      expect(libs).to.equal(expected);
    });
  });
});

Or call it getLibs, doesn't matter to me. We do see some side effects from importing scripts.js but that's a different discussion.

[mokimo]

@JasonHowellSlavin the milo-college project doesn't see a lot of love but is still the go-to for new projects.
I'd love to see a contribution there, seems like a quick and easy performance win.

I'd also suggest sharing this on #milo-dev for more visibility of other projects so they have the opportunity to get this gain.

[Brandon32]

@mokimo We have successfully implemented this on business.adobe.com with adobecom/bacom#220

I can start a contribution to milo-college and share this with #milo-dev.