Skip to content
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

Playground on site not catching invalid Javascript expressions error #1772

Closed
xHomu opened this issue Oct 22, 2021 · 6 comments · Fixed by #1931
Closed

Playground on site not catching invalid Javascript expressions error #1772

xHomu opened this issue Oct 22, 2021 · 6 comments · Fixed by #1931
Labels
🕸 area/website This affects the website 💪 phase/solved Post is done 🐛 type/bug This is a problem

Comments

@xHomu
Copy link
Contributor

xHomu commented Oct 22, 2021

Subject of the issue

Great Job on the v2 rc release! Noticed a minor bug with error catching on the new Playground page for the new Javascript expressions support.

Your environment

Steps to reproduce

Invalid JSX Errors are caught correctly by the Compile tab:

Hello, world!
Below is an example of markdown in JSX.

</div>

image

Errors are NOT caught by the Compile tab when when using the new Javascript expressions syntax {...].

Hello, world!
Below is an example of markdown in JSX.

{r}

The page crashes from the error, error from chrome console:

image

Expected behaviour

Errors within Javascript expressions {...} should caught by the Playground Console tab like JSX syntax error.

Actual behaviour

The page crashes from the error, force you to refresh Playground.

@xHomu
Copy link
Contributor Author

xHomu commented Oct 27, 2021

Adding rehype-sanitize plugin to Playground editor should solve this issue, but would also prevent all Javascript expressions {...} from evaluating.

> import rehypeSanitize from 'rehype-sanitize';

....

          await evaluate(file, {
            ...runtime,
            useDynamicImport: true,
            remarkPlugins,
>            rehypePlugins: [capture('hast'), rehypeSanitize],
            recmaPlugins: [capture('esast')]
          })

@wooorm
Copy link
Member

wooorm commented Oct 28, 2021

rehypeSanitize will remove a bunch of stuff, so that’ll indeed work, but the goal of this playground is to explicitly allow such things. @ChristianMurphy recently touched the playground and I believe also the error catching method, maybe you have ideas Christian?

@ChristianMurphy
Copy link
Member

I added a wrapper that specifically catches and offers recovery for codemirror errors

<ErrorBoundary FallbackComponent={ErrorFallback}>
<CodeMirror {...props} />
</ErrorBoundary>

To catch errors from invalid MDX/JS being run, we'd need to add another ErrorBoundary a few layers up wrapping

export const Editor = ({children}) => {

and figure out what a good error recovery mechanism/UI would look like there.

@wooorm wooorm added 🙉 open/needs-info This needs some more info and removed 🔍 status/open labels Dec 24, 2021
@wooorm
Copy link
Member

wooorm commented Dec 30, 2021

I think this is solved? #1791 (comment)

@shimamooo
Copy link

This isn't solved, editor doesn't have a mechanism to catch invalid expressions. #1791 is related to codemirror state not being in sync with the application state

@wooorm
Copy link
Member

wooorm commented Dec 30, 2021

ah, right, thanks!

@wooorm wooorm changed the title v2 docs Playground not catching invalid Javascript expressions error Playground on site not catching invalid Javascript expressions error Feb 1, 2022
@wooorm wooorm added 👍 phase/yes Post is accepted and can be worked on 🕸 area/website This affects the website and removed 🙉 open/needs-info This needs some more info labels Feb 1, 2022
xHomu added a commit to xHomu/mdx that referenced this issue Feb 3, 2022
wooorm pushed a commit that referenced this issue Feb 7, 2022
Closes GH-1772.

Reviewed-by: Christian Murphy <christian.murphy.42@gmail.com>
Reviewed-by: Titus Wormer <tituswormer@gmail.com>
@wooorm wooorm added 💪 phase/solved Post is done and removed 👍 phase/yes Post is accepted and can be worked on labels Feb 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🕸 area/website This affects the website 💪 phase/solved Post is done 🐛 type/bug This is a problem
Development

Successfully merging a pull request may close this issue.

4 participants