fix broken mdx links #17080
fix broken mdx links #17080
Conversation
☁️ Nx Cloud ReportCI ran the following commands for commit 629f503. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this branch ✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
| @@ -49,8 +49,16 @@ export const CodeOrSourceMdx: FC<CodeOrSourceMdxProps> = ({ className, children, | |||
| ); | |||
| }; | |||
|
|
|||
| function generateHrefWithHash(hash: string): string { | |||
| // eslint-disable-next-line no-undef | |||
| const location = window.parent ? window.parent.location : window.location; | |||
There was a problem hiding this comment.
This is an illegal operation cross origin!
There was a problem hiding this comment.
which can happen when a storybook is composed in:
https://storybook.js.org/docs/react/sharing/storybook-composition#gatsby-focus-wrapper
There was a problem hiding this comment.
@ndelangen Got any suggestions to update the location?
There was a problem hiding this comment.
BTW, your link seems to point to a header called "gatsby-focus-wrapper" but i don't see it in the page
There was a problem hiding this comment.
maybe the check should be Element instead of HTMLElement WDYT?
There was a problem hiding this comment.
I think that the issue is not the check but the fact that we're not passing the correct URL
We are passing only the hash and not appending it to the already existing URL or replacing it with existing hash in the URL.
There was a problem hiding this comment.
When we call addons.getChannel().emit(NAVIGATE_URL, url), where is the actual code that navigating? Maybe we will have access to the full URL there and we can change the logic there.
There's a getBase function in router.tsx
const getBase = () => `${document.location.pathname}?`;What are the implications of document.location vs window.location?
BTW, the links in the side navigation are working perfectly, maybe we can re-use their logic?
| @@ -49,8 +49,16 @@ export const CodeOrSourceMdx: FC<CodeOrSourceMdxProps> = ({ className, children, | |||
| ); | |||
| }; | |||
|
|
|||
| function generateHrefWithHash(hash: string): string { | |||
| // eslint-disable-next-line no-undef | |||
| const location = window.parent ? window.parent.location : window.location; | |||
There was a problem hiding this comment.
which can happen when a storybook is composed in:
https://storybook.js.org/docs/react/sharing/storybook-composition#gatsby-focus-wrapper
|
Thanks @sag1v @ndelangen & apologies for the delay in getting to this I bisected and confirmed that the code broke in I believe the error is in this code: https://github.com/storybookjs/storybook/pull/16440/files#diff-f62af13e39612cc8ebcb82a37a3a55afa29dbea92a0bd94162404a5e7e52aca1R136-R144 And that there's a missing test case which corresponds to |
|
@shilman I'm not sure i understand. I don't think we are even calling |
|
@sag1v You might be right. But it's definitely that PR that broke the behavior. Perhaps it's just how react-router handles navigation vs. reach-router. @ndelangen can you help out here? |
|
@ndelangen Thanks, i can't wait to test it in our project. Just to add my 2 cent, i think this is where things started to break |
|
@shilman thank you! |
|
@sag1v 6.5.0-alpha.x If you can confirm I'll patch it back and release it as part of 6.4.10 |
Issue: #16918
What I did
chained the hash to the location.search instead of just passing it to
navigateI think this is a critical bug fix and should get released ASAP :)