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: report error #183

Merged
merged 2 commits into from
Jun 15, 2023
Merged

Conversation

jansedlon
Copy link
Contributor

Test Plan

Hi! Many times I have experienced a situation where something happened during development and I couldn't see any error.
To give you an example.. I installed lexical and @lexical/react, which is a wysiwyg builder from Facebook. I think they don't provide ESM bundle or whatever. My point is that if you use it anywhere in the code like

import { LexicalComposer } from '@lexical/react/LexicalComposer.js';
console.log(LexicalComposer);

The build starts hanging
image

There is no error to be viewed. I dug deeper and had to debug the code. Turns out, the closeWithGrace is called in tests/mocks/index.ts, it stops the mock server but does nothing else.

Not sure if this PR is the right approach, but at least it will report any errors in the console and closes the whole dev server.

With this change, it looks like this
image

Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I've moved and modified your changes to the server/index.ts which I think is the better place for this. Let me know if this doesn't solve your issue.

Thank you!

@kentcdodds kentcdodds merged commit 0a22a46 into epicweb-dev:main Jun 15, 2023
4 checks passed
@kentcdodds kentcdodds mentioned this pull request Jun 15, 2023
@jansedlon
Copy link
Contributor Author

I don't think that's gonna work, because when I debugged it, the server/index.ts wasn't run, that why I put it in the mock server file. I think I tried your approach at one point and didn't work, but I'm gonna have to double check tomorrow

@kentcdodds
Copy link
Member

Perhaps we need it in both 🤔 Or maybe we should put it in the index.js file in the root. Or all three... It just feels pretty strange putting it in the mocks directory.

@onemen
Copy link
Contributor

onemen commented Jun 17, 2023

In my testing, on Windows 11, in case of an error in mock server or express server closeWithGrace log to the console and close the server however the tsx process does not close. I have to manually close it.

@andrecasal
Copy link
Contributor

In my testing, on Windows 11, in case of an error in mock server or express server closeWithGrace log to the console and close the server however the tsx process does not close. I have to manually close it.

I can confirm this also happens in the latest macOS (Ventura 13.4).

@kentcdodds
Copy link
Member

I've got work locally that puts it in the root index.js and that works for me.

@andrecasal
Copy link
Contributor

andrecasal commented Jun 19, 2023

I've got work locally that puts it in the root index.js and that works for me.

Sorry, I didn't get what you mean. Is having it wrap everything on the index.js enough?

@kentcdodds
Copy link
Member

This: 9543c5f :)

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.

None yet

4 participants