Skip to content

Conversation

@SimonMilord
Copy link
Contributor

@SimonMilord SimonMilord commented Dec 10, 2025

SFINT-6534

⚠️ DO NOT MERGE BEFORE 2026!'

IN THIS PR:

Trying to use the atomic-insight-generate-answer button in an atomic page is giving the following error:

image

The issue:

The build order seems to call the rollup script that copies the assets for the CDN before the assets are loaded/copied in the build:stencil-lit script.

We call build:stencil-lit which includes the rollup -c rollup.config.js which copies the assets for the CDN, but then the build:copy-assets & build:list-assets which loads and saves the sparkles.svg are not yet called. So the CDN does not include the svg.

The solution:

Main Changes in turbo.json:

  • Reorder dependencies to ensure assets get copied and listed before CDN build:
  • build:cem now depends on build:cdn instead of build:stencil-lit
  • build:cdn is a new task that depends on build:list-assets
  • build:list-assets is a new task that depends on build:copy-assets
  • build:copy-assets now depends on both build:stencil-lit and build:locales

Why:
This enforces the required build order:

  • First, run build:stencil-lit & build:locales → generates base files.
  • Then build:copy-assets (copies assets like sparkles.svg).
  • Then build:list-assets (makes a list of all assets, e.g., for documentation or CDN).
  • Finally, build:cdn (uses rollup to bundle CDN assets after everything is copied and listed, ensuring things like sparkles.svg are present).

Additional Cleanups:

  • Removed CDN from the included sources in build:stencil-lit's outputs (so only specified assets/files are included as inputs/outputs).

Main Changes in package.json:

  • Split rollup run from build:stencil-lit and into build:cdn: Previously, build:stencil-lit contained rollup -c rollup.config.js && ... rm-rf ... Now, build:stencil-lit no longer runs rollup, only builds stencil/lit files and does type checks.
  • New script build:cdn added: Runs rollup/copy for CDN, and cleans up loader package.
  • Updated web:cdn command: Now runs turbo run build:cdn && node ./scripts/start-cdn.mjs, connecting the turbo pipeline to the new CD pipeline.

Why:

  • This ensures rollup (the copier for CDN) only runs after assets are present—using the new turbo pipeline built in turbo.json.

Proof it works:

The CDN without the fix:
Screenshot 2025-12-10 at 12 00 05 PM

The CDN with the fix:
Screenshot 2025-12-10 at 12 16 10 PM

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a build order issue where the sparkles.svg asset was missing from the CDN bundle, causing the atomic-insight-generate-answer-button component to fail with a 404 error. The fix moves the asset preparation scripts (copy-assets.mjs and list-assets.mjs) earlier in the build:stencil-lit script execution, ensuring assets are available before the rollup bundler copies them to the CDN directory.

Key Changes

  • Inlined node ./scripts/copy-assets.mjs && node ./scripts/list-assets.mjs into the build:stencil-lit script, positioning them before the rollup -c rollup.config.js step
  • This ensures sparkles.svg and other assets are copied to dist/atomic/assets/ before rollup attempts to bundle them into the CDN

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@SimonMilord SimonMilord marked this pull request as draft December 10, 2025 18:44
@SimonMilord SimonMilord marked this pull request as ready for review December 10, 2025 19:38
@lavoiesl
Copy link
Collaborator

Marking as draft because of the disclaimer:

DO NOT MERGE BEFORE 2026

@SimonMilord SimonMilord marked this pull request as ready for review December 12, 2025 20:57
Copy link
Collaborator

@erocheleau erocheleau left a comment

Choose a reason for hiding this comment

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

If it works, I'm all for it, but it's hard to know it's the best solution with the existing build process.
But to me it looks good

@louis-bompart louis-bompart self-assigned this Jan 5, 2026
@louis-bompart louis-bompart added this to the 1st 2026 release milestone Jan 5, 2026
Copy link
Collaborator

@louis-bompart louis-bompart left a comment

Choose a reason for hiding this comment

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

Overall LGTM, tho questions/changes to be addressed before moving up

@SimonMilord SimonMilord enabled auto-merge January 5, 2026 21:37
@SimonMilord SimonMilord added this pull request to the merge queue Jan 5, 2026
Merged via the queue into main with commit 1228e51 Jan 5, 2026
98 checks passed
@SimonMilord SimonMilord deleted the SFINT-6534 branch January 5, 2026 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants