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

fix: add error boundary to catch invalid code within the document code #1497

Merged

Conversation

LukasGerm
Copy link
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • infrastructure changes
  • Other... Please describe:

What is the current behavior?

nksaraf/vinxi#119

If you add gibberish code to your entry-server.tsx within the document property, the server will crash and you need to manually start it again.

What is the new behavior?

The server will error but not crash. You can fix the issue with your code and just reload your browser.

Other information

nothing to add

Copy link

changeset-bot bot commented May 22, 2024

🦋 Changeset detected

Latest commit: 21af1e2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@solidjs/start Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@ryansolid ryansolid merged commit ea64f7c into solidjs:main May 28, 2024
1 check passed
@ryansolid
Copy link
Member

I think this is the best we have right now. Funny thing is it adds comments around the HTML document which I don't think has any issues but it isn't how I ever expected this to work.

@LukasGerm
Copy link
Contributor Author

@ryansolid Yeah I was not able to get a better working solution at the time. 😄 I mean, if its an issue, we can probably iterate.

@ryansolid
Copy link
Member

No it isn't easy to do. I have an idea of a solution but I basically need to remake a limited version of the errorboundary from scratch..

@ryansolid
Copy link
Member

Looking closer I see this solution doesn't actually show the error but makes the dev server not crash. Let me see if we can do more here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants