Fix sidecar in website
:pushpin: Summary
Fix the sidecar in website by ensuring the layout object exists. While the fix doesn't demystify the root cause it does seem to fix the immediate issue. I suspect a mix of race and frontmatter object override.
To test the fix run ember s --environment=production to get prember to serve static assets. /about/principles is a good page to check the sidecar doesn't render.
:link: External links
Jira ticket: HDS-4770
👀 Component checklist
- [ ] Percy was checked for any visual regression
- [ ] A changelog entry was added via Changesets if needed (see templates here)
:speech_balloon: Please consider using conventional comments when reviewing this PR.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Updated (UTC) |
|---|---|---|---|
| hds-showcase | ✅ Ready (Inspect) | Visit Preview | Apr 10, 2025 0:28am |
| hds-website | ✅ Ready (Inspect) | Visit Preview | Apr 10, 2025 0:28am |
@alex-ju I'm looking at this to see if I remember something more of what I did and why
@alex-ju something very weird is happening here.
Compare the source code:
With how it's compiled:
I need to investigate more...
@alex-ju I went back to a previous commit (https://github.com/hashicorp/design-system/pull/2361 and since then the logic has not been changed); then I looked at how the code would be compiled, and from what I see it's compiled as one would expect:
So it must be something not related to our code but to the compilation itself. Could it be something introduced with the migration to pnpm? @aklkv can you think of something that could change so drastically the code compilation?
@alex-ju I went back to a previous commit (#2361 and since then the logic has not been changed); then I looked at how the code would be compiled, and from what I see it's compiled as one would expect
So it must be something not related to our code but to the compilation itself. Could it be something introduced with the migration to pnpm? @aklkv can you think of something that could change so drastically the code compilation?
The compiled code is the same, from what I've seen, but the computed value for frontmatter is different – I saw the same results in my debugging with the ones you shared above. This makes me think that the issue has to do with various async tasks taking longer or shorter than before (potentially after the pnpm migration), but couldn't get to the bottom of it.
@alex-ju @aklkv I was able to narrow down the PR that introduced this regression and it is this one: https://github.com/hashicorp/design-system/pull/2729
This is the generated code when you check out https://github.com/hashicorp/design-system/pull/2708 which is the previous PR that was committed:
as you can see the hasCover/hasSidecar are correctly evaluated as frontmatter?.layout?.cover/sidecar ?? true and then assigned to this.modelCache[path] as variables (o/r)
This instead is the generated code when you check out https://github.com/hashicorp/design-system/pull/2729:
As you can see, the logical evaluation has disappeared, and the hasCover/hasSidecar are always assigned as true to the this.modelCache[path] object (and in fact in this screen you see the sidecar appear even if it should not).
I am not sure how to debug more this, probably @aklkv may have some ideas of what may be?
PS for context, if I change this line:
const hasSidecar = frontmatter?.layout?.sidecar ?? true;
to this:
let hasSidecar;
if (frontmatter.layout && frontmatter.layout.sidecar === false) {
hasSidecar = false;
} else {
hasSidecar = true;
}
suddenly the compiler changes how the logic is converted to something that "works":
So it must be a compilation problem, not a source code problem.
I finally had time to poke around, I was able to reproduce in current deployed version but if I check my #2810 website preview it seems to be just fine. I wanted to make sure I was not missing anything
@didoo, your hunch was spot on. This type of check:
const hasCover = frontmatter?.layout?.cover ?? true;
was previously affected by a Babel issue. In versions prior to Babel 7.8, the nullish coalescing operator (??) wasn’t fully supported and could be transpiled incorrectly, sometimes behaving like the logical OR operator (||). That means if the left-hand side was false, it would incorrectly fall back to the right-hand side — which is not the expected behavior of ??.
In my branch, I bumped the Babel version (either directly or via a preset upgrade), and the issue resolved itself — everything now behaves correctly.
Gotta love the quirks of transpilers. 😅
I’ve bumped babel to the latest version across all packages as a precautionary measure. #2884
@didoo, your hunch was spot on. This type of check:
const hasCover = frontmatter?.layout?.cover ?? true;was previously affected by a Babel issue. In versions prior to Babel 7.8, the nullish coalescing operator (??) wasn’t fully supported and could be transpiled incorrectly, sometimes behaving like the logical OR operator (||). That means if the left-hand side was false, it would incorrectly fall back to the right-hand side — which is not the expected behavior of ??.
In my branch, I bumped the Babel version (either directly or via a preset upgrade), and the issue resolved itself — everything now behaves correctly.
Gotta love the quirks of transpilers. 😅
That is so fascinating! 😯 🤯
/cc @alex-ju let's wait for #2884 to be merged, and then we can see if some version of this PR changes are still needed (probably not anymore?)
@alex-ju @aklkv the website issue with the sidenav has been fixed by #2884 (thanks @aklkv for the extra investigation) so we can close this PR (and the associated ticket): https://hds-website-git-chore-babel-hashicorp.vercel.app/about/principles