Skip to content

Docs: ensure stackblitz.js loads conditionally as intended#41482

Merged
julien-deramond merged 2 commits intomainfrom
main-lmp-astro-fixes
May 23, 2025
Merged

Docs: ensure stackblitz.js loads conditionally as intended#41482
julien-deramond merged 2 commits intomainfrom
main-lmp-astro-fixes

Conversation

@louismaximepiton
Copy link
Member

@louismaximepiton louismaximepiton commented May 22, 2025

Description & Motivation & Context

During backporting the Astro commit to our doc, there was an issue regarding the Scripts bundled by Astro. I couldn't spot it on Bootstrap side but on our side. The stackblitz.js is loaded on the landing page (main branch) and not the application.js contrary at what is specified in Scripts.astro.

Special thanks to @vprothais !

Type of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would change existing functionality)

Checklist

  • I have read the contributing guidelines
  • My code follows the code style of the project (using npm run lint)
  • My change introduces changes to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

Live previews

Related issues

NA

@louismaximepiton louismaximepiton changed the title Small Astro fixes Docs: Small Astro fixes May 22, 2025
@github-project-automation github-project-automation bot moved this to To do in v5.3.7 May 22, 2025
@julien-deramond julien-deramond moved this from To do to Needs review in v5.3.7 May 22, 2025
<script src="../assets/search.js"></script>

{layout === 'docs' && <script src="../assets/stackblitz.js" />}
{layout === 'docs' && <DocsScripts />}
Copy link
Member

@julien-deramond julien-deramond May 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch @louismaximepiton — and nice patch here Orange-OpenSource/Orange-Boosted-Bootstrap@a91f7d3 @vprothais! I'll add both of you in the final commit as co-authors.

I can confirm that prior to your change, adding a simple console.log() in assets/stackblitz.js shows it being loaded on the homepage, docs, and examples — which it shouldn't. That confirms the script is being included globally, and the conditional logic isn't working as intended.

I haven't reviewed the entire Astro issues, but this one seems closely related: withastro/astro#13550. It mentions:

When you put it in a separate component, the script belongs permanently with that component. Then you are conditionally rendering the component here. That aligns with how Astro works.

This lines up well with the behavior we're seeing — so yes, moving this into a component makes sense.

Could you please extract the other fix @louismaximepiton into a separate PR? That way I can merge this one right away, and review the second fix independently later on. 🙏

@julien-deramond julien-deramond moved this from Needs review to Review in progress in v5.3.7 May 22, 2025
@julien-deramond julien-deramond self-requested a review May 23, 2025 07:42
@julien-deramond julien-deramond changed the title Docs: Small Astro fixes Docs: ensure stackblitz.js loads conditionally as intended May 23, 2025
@julien-deramond julien-deramond moved this from Review in progress to Ready to merge in v5.3.7 May 23, 2025
@julien-deramond julien-deramond merged commit 1c3b53b into main May 23, 2025
14 checks passed
@julien-deramond julien-deramond deleted the main-lmp-astro-fixes branch May 23, 2025 07:47
@github-project-automation github-project-automation bot moved this from Ready to merge to Done in v5.3.7 May 23, 2025
Copy link

@Sahil3378 Sahil3378 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const eventRegistry = {};

let uidCounter = 1;
function makeEventUid(element) {
  return element.uidEvent || (element.uidEvent = `uid-${uidCounter++}`);
}

function getElementEvents(element) {
  const uid = makeEventUid(element);
  element.uidEvent = uid;
  eventRegistry[uid] = eventRegistry[uid] || {};
  return eventRegistry[uid];
}

function hydrateObj(obj, props) {
  for (const key in props) {
    obj[key] = props[key];
  }
}

const EventHandler = {
  off: function (element, eventType, fn) {
    element.removeEventListener(eventType, fn);
  }
};

function bootstrapHandler(element, fn) {
  return function handler(event) {
    hydrateObj(event, {
      delegateTarget: element
    });
    if (handler.oneOff) {
      EventHandler.off(element, event.type, fn);
    }
    return fn.apply(element, [event]);
  };
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants