Skip to content

Fix sidecar in website #2815

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix sidecar in website #2815

wants to merge 1 commit into from

Conversation

alex-ju
Copy link
Member

@alex-ju alex-ju commented Apr 10, 2025

📌 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.

🔗 External links

Jira ticket: HDS-4770


👀 Component checklist

💬 Please consider using conventional comments when reviewing this PR.

@alex-ju alex-ju requested a review from a team as a code owner April 10, 2025 12:28
Copy link

vercel bot commented Apr 10, 2025

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 alex-ju added the docs-website Content updates to the documentation website label Apr 10, 2025
@didoo
Copy link
Contributor

didoo commented Apr 15, 2025

@alex-ju I'm looking at this to see if I remember something more of what I did and why

@didoo
Copy link
Contributor

didoo commented Apr 15, 2025

@alex-ju something very weird is happening here.

Compare the source code:
modified

With how it's compiled:
modified

I need to investigate more...

@didoo
Copy link
Contributor

didoo commented Apr 15, 2025

@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:
modified

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
Copy link
Member Author

alex-ju commented Apr 16, 2025

@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.

@didoo
Copy link
Contributor

didoo commented Apr 16, 2025

@alex-ju @aklkv I was able to narrow down the PR that introduced this regression and it is this one: #2729

This is the generated code when you check out #2708 which is the previous PR that was committed:
modified

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 #2729:
modified

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":
modified

So it must be a compilation problem, not a source code problem.

@heatherlarsen heatherlarsen mentioned this pull request Apr 17, 2025
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-website Content updates to the documentation website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants