move markdown-toolbar-element into unconditional render #2353
Conversation
|
size-limit report 📦
|
| @@ -1,4 +1,5 @@ | |||
| import React, {forwardRef, useImperativeHandle, useRef, useEffect} from 'react' | |||
| import React, {forwardRef, useImperativeHandle, useRef} from 'react' | |||
| import '@github/markdown-toolbar-element' | |||
There was a problem hiding this comment.
Just wanted to double-check, I think one intent behind the condition is to allow for SSR. With this change, I believe SSR would no longer work because @github/markdown-toolbar-element references globals like HTMLElement and tries to register itself using if (!window.customElements.get('...')) when imported.
There was a problem hiding this comment.
That's correct. I think as we look more towards SSR we can look to centralise these dependencies into a client-side only bundle.
* Don't crash when building docs * Remove import again after bad merge * Replace require call with a dynamic import * Disable prettier rule * Revert "Replace require call with a dynamic import" This reverts commit 4247039. See nodejs/node#35889 * Wrap `@github/markdown-toolbar-element` require in `useEffect` Co-authored-by: Josh Black <[email protected]> * Don't evaluate on every render Co-authored-by: Matthew Costabile <[email protected]> Co-authored-by: Josh Black <[email protected]> Co-authored-by: Matthew Costabile <[email protected]>
|
Hi! This pull request has been marked as stale because it has been open with no activity for 60 days. You can comment on the pull request or remove the stale label to keep it open. If you do nothing, this pull request will be closed in 7 days. |
github-actions
bot
deleted the
move-markdown-toolbar-element-into-uncodnitional-render
branch
This moves the import of
markdown-toolbar-elementout of a conditional, which seems to be causing SegFaults due to poor support of ESM modules in various tools like Jest. Moving the import out of the conditional allows these tools to pick up the import which resolves segfaults.We may want to move all web component definitions to a central file;
github/githubcould de-duplicate these imports as it already imports them via PVC or via direct imports.Closes #2339, refs #2334
Merge checklist
Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.