Skip to content

Conversation

@aklkv
Copy link
Collaborator

@aklkv aklkv commented Apr 22, 2025

📌 Summary

Today we are referencing <LinkToExternal/> component and in the many cases it doesn't exists unless consumer has ember-engines as dependancy. In the embroider/vite world all components are evaluated at the build time and needs to resolve correct dependancy. If we were to run hds embroider/vite today it would result in an error about unresolved LinkToExternal component. In order to make this behaviour more controlled we are encapsulating requirements for ember-engines to be explicitly required only in situation when we pass @isRouteExternal as true in which case we do our check for required dependancy resigne component instance or through an error.

Shipping ember-engines is not an option for a couple of reasons:

  • it has very interesting v1 era requirements which are impossible to satisfy in addon structure (nor should we)
  • it is consumer land dependancy and we should leverage this tacktics of not locking users to our version and letting consumer provide own copy only in situation when it's needed. I can see other optimizations in similar fashion but this is the most pressing one at the moment

🛠️ Detailed description

📸 Screenshots

🔗 External links

Jira ticket: HDS-XXX
Figma file: [if it applies]


👀 Component checklist

💬 Please consider using conventional comments when reviewing this PR.

@aklkv aklkv self-assigned this Apr 22, 2025
@vercel
Copy link

vercel bot commented Apr 22, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
hds-showcase ✅ Ready (Inspect) Visit Preview May 1, 2025 8:21am
hds-website ✅ Ready (Inspect) Visit Preview May 1, 2025 8:21am

@github-actions
Copy link
Contributor

github-actions bot commented Apr 22, 2025

📦 RC Packages Published

Latest commit: 368e18a

Published 1 packages

@hashicorp/[email protected]

yarn up -C @hashicorp/design-system-components@rc

@aklkv aklkv mentioned this pull request Apr 22, 2025
2 tasks
@aklkv aklkv marked this pull request as ready for review April 22, 2025 22:46
@aklkv aklkv requested a review from a team as a code owner April 22, 2025 22:46
@aklkv
Copy link
Collaborator Author

aklkv commented Apr 22, 2025

tested on:

  • cloud-ui - embroider/webpack + ember-engines
  • terraform-enterprise - embroider/vite no ember-engines

zamoore
zamoore previously approved these changes Apr 30, 2025
"ember-source": "^3.28.0 || ^4.0.0 || ^5.3.0",
"ember-engines": ">= 0.11.0"
},
"peerDependenciesMeta": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to be safe, did you try to install on a codebase without ember-engines?

Copy link
Contributor

Choose a reason for hiding this comment

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

@aklkv aklkv merged commit 12919f3 into main May 1, 2025
16 checks passed
@aklkv aklkv deleted the fix/link-to-external-retake branch May 1, 2025 17:12
@hashibot-hds hashibot-hds mentioned this pull request May 1, 2025
@alex-ju alex-ju added this to the [email protected] milestone May 7, 2025
shleewhite pushed a commit that referenced this pull request May 7, 2025
@aklkv aklkv restored the fix/link-to-external-retake branch May 10, 2025 04:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

packages/components release-candidate Publishes release candidates to npm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants