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

Decouple lottie-web in onboarding components #6201

Open
2 of 4 tasks
elycheea opened this issue Oct 9, 2024 · 3 comments
Open
2 of 4 tasks

Decouple lottie-web in onboarding components #6201

elycheea opened this issue Oct 9, 2024 · 3 comments
Assignees
Labels
area: plg ✨ Product-led growth, includes novice to pro dependencies Pull requests that update a dependency file role: dev
Milestone

Comments

@elycheea
Copy link
Contributor

elycheea commented Oct 9, 2024

Follow up to #3959.

The original issue was to investigate issues with tree shaking. In our initial review, we weren’t able to replicate the issue where lottie-web was not being shaken, but we’ve continued to see reports of inflated builds1.

The problem

Adopters of our library would like to leverage our package without inflated bundle sizes that have resulted from the inclusion of lottie-web as a dependency. In some cases, lottie-web remains even after tree-shaking, which has resulted in developers taking a more fragile approach to preventing it from being bundled2.

More recently, we’ve seen additional contributions3 which include Lottie animations. Before we introduce these new patterns, we’d like to explore new ways to allow our components and library to support these animations while reducing the overhead, risk, and performance issues passed on to our consuming teams.

Solution

In a basic way, decoupling lottie-web from the set of onboarding components — however, this poses some risk to teams that may already be consuming them using Lottie animations. In recent reviews of our telemetry, Coachmarks and InterstitialScreen are known to be among the higher used onboarding components, however, they are still considered experimental/preview components.

Currently, lottie-web is imported in SteppedAnimatedMedia which is in turn used in

  • CoachmarkOverlayElements
  • CoachmarkStack
  • InlineTip
  • and InterstitialScreen

All of these components accept a media prop which will leverage the SteppedAnimatedMedia when passed filePaths.

Tasks

Preview Give feedback
  1. 0 of 3
    needs: investigation needs: prototype role: dev
    amal-k-joy
  2. component: Coachmark component: InlineTip component: Interstitial screen role: dev type: enhancement 💡
    amal-k-joy
  3. component: Coachmark component: InlineTip component: Interstitial screen role: dev
    amal-k-joy

Footnotes

  1. Kindly documented by @Mikadv: https://ibm-studios.slack.com/archives/C013ZTX0N6B/p1725456070249159?thread_ts=1725374600.890359&cid=C013ZTX0N6B

  2. Also from @asfordmatt: https://ibm-studios.slack.com/archives/C013ZTX0N6B/p1725983300345619?thread_ts=1725374600.890359&cid=C013ZTX0N6B

  3. Animated headers from @glapadre @mikeolasov @marion-bruells

@github-project-automation github-project-automation bot moved this to Needs triage 🧐 in Carbon for IBM Products Oct 9, 2024
@elycheea elycheea added role: dev area: plg ✨ Product-led growth, includes novice to pro dependencies Pull requests that update a dependency file and removed status: needs triage 🕵️‍♀️ labels Oct 9, 2024
@elycheea elycheea moved this from Needs triage 🧐 to Needs refinement 🤓 in Carbon for IBM Products Oct 9, 2024
@ljcarot ljcarot moved this from Needs refinement 🤓 to In progress in Carbon for IBM Products Oct 30, 2024
@tomsoal
Copy link

tomsoal commented Dec 17, 2024

Any update on this issue please? We've had security vulnerabilities flagged up in our UI code due to the presence of eval() in lottie-web on this line. This specific example is pointing to this CWE

If this package was decoupled from within this repo we could see this vulnerability disappear which would save us having to make any temporary workarounds - thank you

@elycheea elycheea added this to the 2025 Q1 milestone Dec 17, 2024
@elycheea
Copy link
Contributor Author

@tomsoal We were currently targeting February for its actual removal, but think we can escalate this with the CWE. 🙏 CC @ljcarot @matthewgallo @amal-k-joy

@ljcarot
Copy link
Member

ljcarot commented Dec 18, 2024

This currently is unassigned @elycheea Do you want to assign it to someone?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: plg ✨ Product-led growth, includes novice to pro dependencies Pull requests that update a dependency file role: dev
Projects
Status: In progress
Development

No branches or pull requests

4 participants